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

feat: change sort col using two fixed keys #1256

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

Conversation

mlosicki
Copy link

My attempt at this feature request: #1000

Description

Change the sort col using two fixed keys, < and >.
q changes the sort order. The behaviour is now similar to top

Demo

asciicast
(The player doesn't show the key inputs sadly)

Implementation details

  • Used the q key instead of r because it is already used in some places (e.g. node drain). I assumed you do not want to change the default keybindings even though I am personally in favour of refactoring with r
  • I believe the sort implementation could benefit from a refactor. Currently, the values are sorted on the displayed string value. I took the inspiration from age to fix sorting on capacity, but it isn't ideal. It can lead to issues like this too: Support sort by revision #885. The original value or a specific compare function should be kept on the level of the Fields or even Row.
  • Not a Go dev, sorry upfront for non-idiomatic code

Closes #1000

Change the sort col using two fixed keys instead of
using the fixed bindings per column. Another fixed key is used
to change the sort order. The behaviour is now similar to `top`

The implementation is based on re-evaluating which columns of the
model should be visible, instead of checking the currently displayed
columns. The displayed column names might be decorated with extra
content which make string comparisons cumbersome.

Also, fix the sorting on CAPACITY-like columns (e.g. pv, pvc) by
comparing the values as a `Quantity` and not `strings`.
@chauncey-garrett
Copy link

This is a great improvement! Here's to hoping it can get merged.

@olee
Copy link

olee commented Jun 13, 2024

What's the status of this? Any reason it couldn't be merged? In my case I'd love to order by assigned memory limit (not the percentage) which is not possible atm.

@ventsislav-georgiev
Copy link

Rebased with small fixes here: #2739

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

Successfully merging this pull request may close these issues.

TOP like sort shortcuts.
4 participants