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

Enhance rename prompt #1162

Merged
merged 5 commits into from
Mar 20, 2023
Merged

Enhance rename prompt #1162

merged 5 commits into from
Mar 20, 2023

Conversation

rrveex
Copy link
Contributor

@rrveex rrveex commented Mar 17, 2023

Argument for rename to work like this:

map r rename       # original rename
map i rename start # place the cursor at the beginning
map a rename ext   # place the cursor on the last .
map w rename cw    # no original name in prompt

@joelim-work
Copy link
Collaborator

Hi @rrveex , thanks for your pull request!

I'm just wondering, if you just want to create some more convenient rename mappings for yourself, would it not be possible to achieve this entirely within configuration and without having to make changes to the code?

map i :rename; cmd-home
map a :rename; cmd-word-back; cmd-left  # hacky way to move to the last '.', but it should work for most cases
map w :rename; cmd-delete-home

On the other hand, if you actually want to add these enhancements so that it can be used by others, this would require more consideration as to whether it's worth adding in or not. There is also the possibility where other users might wish to add in their own variations of rename that are not already covered here, for example keeping the extension but deleting the stem (i.e. if the file to be renamed is foo.txt, then the command line will look like |.txt).

@rrveex
Copy link
Contributor Author

rrveex commented Mar 19, 2023

should work for most cases

That's exactly what I'm not happy with.

whether it's worth adding in or not.

My thought was "it doesn't break anything so why not?". Actually it costs maintenance in the long run. So if most users are happy with "works in most cases", you're right, it's not worth adding and wasting time maintaining a bigger code base.

other users [...] add their own variations of rename

I should have opened an issue instead of a PR, sorry.

@rrveex rrveex closed this Mar 19, 2023
@joelim-work
Copy link
Collaborator

So just to be clear, I am not against your change - I am just cautious about introducing new features when there are possible alternatives. As you have said, every new feature adds to the maintenance cost.

Please feel free to open an issue, create a discussion or otherwise just reopen this pull request. I do acknowledge that currently lf has no built-in way to place the cursor at the extension, and if there is enough support from the community then I can see a possibility that this change can be accepted. In any case, the decision for that lies with the project owner, which is not me.

@gokcehan
Copy link
Owner

@rrveex @joelim-work There was indeed already a discussion about this in #279 and I had agreed to place the cursor at the extension by default so other commands can be easily defined but nobody sent a PR afterwards.

@rrveex rrveex reopened this Mar 20, 2023
@rrveex
Copy link
Contributor Author

rrveex commented Mar 20, 2023

@gokcehan There was indeed already a discussion about this in #279 and I had agreed to place the cursor at the extension by default so other commands can be easily defined but nobody sent a PR afterwards.

Reopened with cursor default on dot if there is an extension, at the end otherwise.

@gokcehan
Copy link
Owner

@rrveex Looks good, thanks for the patch.

@gokcehan gokcehan merged commit 122dee1 into gokcehan:master Mar 20, 2023
@rrveex rrveex deleted the rename_ui branch March 20, 2023 18:11
@ilyagr
Copy link
Collaborator

ilyagr commented Mar 26, 2023

Nice feature, thanks!

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

4 participants