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 support for moving to a specific line #1196

Merged
merged 2 commits into from Apr 8, 2023
Merged

Add support for moving to a specific line #1196

merged 2 commits into from Apr 8, 2023

Conversation

joelim-work
Copy link
Collaborator

@joelim-work joelim-work commented Apr 4, 2023

Fixes #240
Fixes #1033

I know there's been a few attempts in the past to implement this, such as #242 and #1040 and possibly others that I have missed. I'm not sure why those attempts appear to have been stalled, I suppose although setting dir.ind is straightforward, the difficult part is determining how to calculate dir.pos so that the amount of scrolling is minimised.

It looks possible to simply use the existing up/down functions to move to a new line (no scrolling happens if the destination is already visible, so the effect is smoother). This is already used in other functions like findNext/findPrev and searchNext/searchPrev, and I think it's better to recycle the scrolling logic in the up/down functions rather write it from scratch.

@shabahengam
Copy link

a bit offtopic but after seeing all of your great PRs I think you can get some idea from this project https://github.com/jacokoo/fff
I mentioned this project before in another PR and I hope @gokcehan don't get mad at me for mentioning this project again :) but I wish someone could implement some features of that project like jump,filter by size etc.. in lf
btw thanks for your efforts to improve lf and also thanks to @gokcehan for creating such a great project

@gokcehan
Copy link
Owner

gokcehan commented Apr 8, 2023

@joelim-work Looks good to me. I think I wasn't sure about the parent movement commands in #242 and use of regexp in #1040 for this purpose. I'm not sure how the custom mappings proposed for parent movement in #242 works in practice, so if there is a reason to include them as well, feel free to discuss it in another issue or patch. Thanks for the patch.

@shabahengam Feel free to mention anything you want :) I'm sorry if I got mad for such a thing before. I try to be more inclusive towards new features these days as I accepted myself as a minority among lf users.

@gokcehan gokcehan merged commit b5c83db into gokcehan:master Apr 8, 2023
3 checks passed
@joelim-work joelim-work deleted the move-to-index branch April 9, 2023 02:13
@joelim-work
Copy link
Collaborator Author

@gokcehan For the parent movement commands, I agree with your comment in #242 (comment) that it's better to avoid implementing them as builtin commands if possible, to keep the overall lf codebase small. My opinion is that instead of implementing every operation natively in the code, lf should expose low-level operations and leave it up to users to compose higher-level operations out of them.

@shabahengam Deciding on whether a feature should be included in a project is not an easy task in general. Every new feature introduces additional complexity and maintenance burden, but will likely benefit only a subset of users. I believe everyone is welcome to propose new features - just create an issue which includes something like Feature request: in the title, and anyone who is interested will be able to provide you with some feedback.

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.

Is there a way to move the cursor to the nth file in the current directory? Jump to specific line
3 participants