Should z[ and z] reverse and remove existing sort criteria?

[midichef]

When I'm sorting by more than one column, I often want to reverse one of the sort directions. For example, if I am sorting movies by the multiple criteria of 1) genre 2) rating, and 3) title, I may want to see the ratings ascending or descending, but keep the other columns unchanged. Right now the only way to change the direction is to rebuild the sort all over, which is inconvenient.

I've prototyped a version that makes it easy. If you sort a column by z[, it'll be added as usual. If you then hit z] it'll reverse the sort direction, but keep the other columns and keep the column's priority unchanged.

What do you think, should this be the default behavior of z[?

Another change this makes: if you use z[ on a column that's already sorted ascending (or z] on one that's descending), the column is removed from the sort criteria. So to demonstrate with the movie example, let's say I am sorting by ↑genre (ascending), ↡rating (descending), ⇞title (ascending). If I hit z[ on the genre column, it'll only sort by ↓rating and ↟title. If I then hit z] on the rating column, it'll only sort by ↑title. Is this removal behavior intuitive or not?

Here is code to add to your .visidatarc to try it out. I welcome feedback.

@Sheet.api
def orderBySwitchable(sheet, *cols, reverse=False):
    'Add *cols* to internal ordering and re-sort the rows accordingly.  Pass *reverse* as True to order these *cols* descending.  If any of *cols* are already in the ordering with the same direction as *reverse*, remove them. If they have a different direction from *reverse*, flip their sort direction, keeping their place in the ordering.  Pass empty *cols* (or None for cols[0]) to clear internal ordering.'
    if options.undo:
        vd.addUndo(setattr, sheet, '_ordering', copy(sheet._ordering))
        if sheet._ordering:
            vd.addUndo(sheet.sort)
        else:
            vd.addUndo(setattr, sheet, 'rows', copy(sheet.rows))

    do_sort = False
    if not cols or cols[0] is None:
        sheet._ordering.clear()
        cols = cols[1:]
        do_sort = True

    new_ordering = []
    seen_cols = []
    for c, old_reverse in sheet._ordering:
        changed = False
        if isinstance(c, str):
            colname = c
            c = sheet.column(colname)
        else:
            colname = None
        if c in cols:
            if reverse != old_reverse:
                new_ordering.append((colname or c, reverse))
            # if the column's new sort direction is the same as before, it will be removed from _ordering
            changed = True
            seen_cols.append(c)
            do_sort = True
            break
        if not changed:
            new_ordering.append((colname or c, old_reverse))
    unseen_cols = [ col for col in cols if not col in seen_cols ]
    if unseen_cols:
        do_sort = True
        for col in unseen_cols:
            new_ordering.append((col, reverse))
    sheet._ordering = new_ordering
        
    if do_sort:
        sheet.sort()

# replace existing sort criteria
Sheet.addCommand('[', 'sort-asc', 'orderBySwitchable(None, cursorCol)', 'sort ascending by current column; replace any existing sort criteria')
Sheet.addCommand(']', 'sort-desc', 'orderBySwitchable(None, cursorCol, reverse=True)', 'sort descending by current column; replace any existing sort criteria ')
Sheet.addCommand('g[', 'sort-keys-asc', 'orderBySwitchable(None, *keyCols)', 'sort ascending by all key columns; replace any existing sort criteria')
Sheet.addCommand('g]', 'sort-keys-desc', 'orderBySwitchable(None, *keyCols, reverse=True)', 'sort descending by all key columns; replace any existing sort criteria')

# add to existing sort criteria
Sheet.addCommand('z[', 'sort-asc-add', 'orderBySwitchable(cursorCol)', 'sort ascending by current column; add to existing sort criteria')
Sheet.addCommand('z]', 'sort-desc-add', 'orderBySwitchable(cursorCol, reverse=True)', 'sort descending by current column; add to existing sort criteria')
Sheet.addCommand('gz[', 'sort-keys-asc-add', 'orderBySwitchable(*keyCols)', 'sort ascending by all key columns; add to existing sort criteria')
Sheet.addCommand('gz]', 'sort-keys-desc-add', 'orderBySwitchable(*keyCols, reverse=True)', 'sort descending by all key columns; add to existing sort criteria')

[saulpw]

I think this behavior is intuitive and desirable in an interactive context. However, it's tricky to change behavior based on existing column state, as this can make cmdlog replay a lot less predictable. We already have this with e.g. key-col and it's caused some problems. Maybe the ultimate answer is to have key-col put a more precise command like key-col-on or key-col-off (please help come up with better longnames if we do this) on the cmdlog. These sort commands could then do the same.

This would also be better, as we wouldn't be changing the semantics of the existing longnames, but only the bindings to these new commands with new longnames.