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

Add jumps command for displaying the jump list #1233

Merged
merged 3 commits into from May 13, 2023
Merged

Add jumps command for displaying the jump list #1233

merged 3 commits into from May 13, 2023

Conversation

joelim-work
Copy link
Collaborator

I thought it would be nice to add more functionality for displaying the internal state, now that there is some support for piping data into a pager. I have tried to keep the implementation similar to the Vim counterpart, using relative numbers (useful for jumping with a count, such as 3[), as well as displaying a > for the current location in the jump list.

@ilyagr
Copy link
Collaborator

ilyagr commented May 11, 2023

I think this is a very nice idea. Thanks!

In the docs, we could mention how this is supposed to be used. You could add: Each location is marked with the count that can be used with the '[' and ']' commands..

I'm also thinking that it would be good to reverse the output. If the jump list is longer than the screen, the user probably wants to see the current position (which is usually the last position in the jump list) immediately. Currently, they'd have to scroll to the very bottom to see it.


For reference, the output currently looks like this:

Usual case:

  jump  path
  9     /home/ilyagr/dev/lf
  8     /home/ilyagr/dev
  7     /home/ilyagr
  6     /home
  5     /
  4     /usr
  3     /usr/lib
  2     /usr/lib/gcc
  1     /usr/lib
> 0     /usr

After a few [s:

  jump  path
  4     /home/ilyagr/dev/lf
  3     /home/ilyagr/dev
  2     /home/ilyagr
  1     /home
> 0     /
  1     /usr
  2     /usr/lib
  3     /usr/lib/gcc
  4     /usr/lib
  5     /usr

@joelim-work
Copy link
Collaborator Author

Hi @ilyagr, thanks for your comments!

I've updated the documentation to mention the jump counts.

Regarding the order for which the jump list locations are printed, I do agree that it's more user-friendly to print the recent locations first. However Vim uses the opposite order - I'm sort of undecided here but I decided to reverse the order as you suggested anyway. This is done in b26324e, the commit can be reverted if anyone doesn't like it.

BTW the following is sort of off-topic and probably should take place in a separate discussion, but I do recall you wanting to refactor the code for handling the pager - have you made any progress on it? I was reading the code so I could implement jumps, and I'm not exactly a fan of how the runShell function has evolved. The original purpose of runShell was that you would specify what shell command you wanted to run, but for all the pager stuff, the command is fixed and instead you're specifying what data you want to pipe. I was thinking of just creating a separate function for this - maybe runPager, but probably there's a better name.

@ilyagr
Copy link
Collaborator

ilyagr commented May 11, 2023

Thanks!

I do agree that it's more user-friendly to print the recent locations first

I think this is more important than matching what Vim does exactly. In any case, thanks for changing it, and let's see if anybody dislikes the reversed order.

I do recall you wanting to refactor the code for handling the pager - have you made any progress on it?

Yes, I have a version of it from a while ago that I need to freshen up. Sorry for the delay.

@joelim-work
Copy link
Collaborator Author

Yes, I have a version of it from a while ago that I need to freshen up. Sorry for the delay.

That's OK, you can take your time, no need to rush for this kind of thing.🙂

I'm just wondering about other ways this power can be harnessed. In general the idea of having lf print parts of its internal state could be very useful. This jumps command is one such application (AFAIK there's no other way to view the jump list), and I'm sure others will be able to come up with ideas that I would never be able to foresee myself.

@gokcehan
Copy link
Owner

Looks good, thank you @joelim-work for the patch and thank you @ilyagr for the review and suggestions.

@gokcehan gokcehan merged commit 24e224c into gokcehan:master May 13, 2023
3 checks passed
@joelim-work joelim-work deleted the jumps-command branch May 14, 2023 01:43
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.

None yet

3 participants