Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sort-] allow z[ and z] to reverse col sort dir or ignore col #2688

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

midichef
Copy link
Contributor

This PR is a modified version of a feature I prototyped in #1877.

It lets z[ and z] modify a column for sorting, while keeping the existing sort criteria if possible. This is very convenient when exploring data.

There are three cases for these commands:

  1. add column to sort criteria (this is the only use for z[ and z] in current visidata)
    This PR adds new behavior only for the other two cases, where the column is already in the sort criteria:
  2. toggle off the column's sorting
    When the column is used for sorting, and the command sort direction matches the existing direction. Like z[ for a column that is already in ascending order due to a previous z[. The column will be removed from the sort criteria. Any existing sort criteria on columns that are higher in priority, is kept. Lower priority columns are dropped from the sort criteria.
  3. reverse the column's sort direction
    When the column is used for sorting, and the new direction is the opposite of the existing direction. Like z[ for a column that is in descending order from a previous z]. The column will reverse its sort direction. Existing sort criteria are handled the same as in case 2.

(In current visidata, for cases 2 and 3, the column is added to the sort criteria again, uselessly, which is a bug that throws off the sort arrow indicators for future columns used for sorting.)

They now modify the sort order of one column, while preserving sort
criteria for columns that had higher priority.
@midichef
Copy link
Contributor Author

However, in the original discussion of this feature, @saulpw noted a complication in replay when commands depend on state in the sheet:

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.

I implemented this suggestion for key-col in #2622. That PR converts ! into the state-independent key-col-on or key-col-off for the command log. But that method won't work for z[, because one z[ command can modify several columns at once. In that situation, z[ would need to generate multiple sort-asc and sort-asc-add/sort-desc-add commands. Multiple commands from a single command would cause inconsistent state in undo, because undo assumes that commands are undone one at a time:

del sheet.cmdlog_sheet.rows[row_idx]

So how should we handle state-dependent commands here? What specific problems were caused for cmdlog replay? Are these kinds of problems still present? I know some replay behavior and timing has been changed since v2.11.

I find this feature convenient enough that I'd rather have it, even if it does complicate cmdlog replay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants