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

invert-below command to substitute for visual mode #1101

Merged
merged 1 commit into from Feb 25, 2023

Conversation

ilyagr
Copy link
Collaborator

@ilyagr ilyagr commented Feb 3, 2023

This command inverts the selection for the current file and all the files below it. Using this command twice substitutes for having a "visual" mode. This is described in more detail in the docs and in
#477 (comment).

Since invert is bound to v by default, invert-below gets V as a default binding. Update: There is no longer a default binding, you will have to add something along the lines of map V invert-below to lfrc yourself.

Fixes #477. While that's not exactly what was asked for, I hope it's a suitable replacement and seems more in the spirit of lf.

@DusanLesan
Copy link

How about instead of doing this you use logic to select all between nearest selected and current focused file? That would work in both directions. Also that could allow to make commands like jump n items up or down and focus in between. The use case from this PR could be covered by selecting starting point, jumping to bottom and focus all in between.

@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 4, 2023

How about instead of doing this you use logic to select all between nearest selected and current focused file?

It would bother me that if I'm between two selections, I would have to count which is closer.

invert-below allows more complicated behaviors, like adding a block of files to a preexisting selection. You could select a block of files, skip some, and then select another block. Less intuitively, it also works perfectly if you are doing it in reverse order (select a block, then move up, then select another block).

Also that could allow to make commands like jump n items up or down and focus in between.

It's true that repeating invert-below isn't very useful. If none of the files are already selected how about 10 <space>? You can also bind a key to toggle; up.

Aside: I also tried creating a command with cmd tup :toggle; up, but I couldn't get a count to work with a command.

@DusanLesan
Copy link

It would bother me that if I'm between two selections, I would have to count which is closer.

That sounds like a edge-case that could be handled by always using the above file as starting point

This was just an idea that sounded better to me but I cannot speak about implementation so I am fine with this too.

@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 5, 2023

That sounds like a edge-case that could be handled by always using the above file as starting point

I don't mean that it's difficult to implement. I mean that, as a user, if I'm between two selections, I'd have to either guess or count to know which way it would go.

@DusanLesan
Copy link

DusanLesan commented Feb 5, 2023

That sounds like a edge-case that could be handled by always using the above file as starting point

I don't mean that it's difficult to implement. I mean that, as a user, if I'm between two selections, I'd have to either guess or count to know which way it would go.

I think only checking first and last is enough. If current focus is above first selection, select in between and same for the last selection and focus below. If focus is in between always focus all between first above item selected and focus or refuse action

@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 5, 2023

I think it's much simpler to have invert-below that doesn't have those edge cases and works the same every time. It also seems to me to be more powerful, as I explained in my original reply.

It's true that sometimes a solution with edge cases is necessary, but it would need to have some great advantage over a simpler solution to be worth it.

@DusanLesan
Copy link

I think it's much simpler to have invert-below that doesn't have those edge cases and works the same every time. It also seems to me to be more powerful, as I explained in my original reply.

The behavior I explained would be consistent if implemented like that. Like I said I think this addition is good and I just thought that it looked too much like flipping the bits until you get desired selections instead of doing what sounded more intuitive to me.

Thank you for your time

@laktak
Copy link
Contributor

laktak commented Feb 6, 2023

"invert below" is such a hard to explain concept. While I see how it could be useful (after reading the linked comments) I don't think it is very intuitive.

Please don't map it by default.

@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 12, 2023

I've been a bit quiet here since I don't have much to add, but maybe I'll write down where I'm at. @laktak, I'm not convinced -- I think having this feature and mapping this to V does much more good than harm.

I am not particularly certain of my opinion, however. If the consensus and/or @gokcehan's opinion is not to merge this, or to merge it without a binding, that's OK with me.

Most importantly, I haven't thought of a way to make it better or more intuitive than it already is without much larger changes to lf.

Update: I thought I'd ask in the linked bug whether people who've been wanting visual selection find this useful or not.

@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 15, 2023

I did think of another design, see #477 (comment). However, I'm not sure how interested I am in implementing it, since this PR is good enough for me.

Anybody is welcome to it, if they are interested. I would appreciate a warning so that I don't start implementing it when somebody else already started on it.

@gokcehan
Copy link
Owner

@ilyagr Thanks for working on this. Last time I checked this PR, I decided to leave it open for more discussion as I didn't have much to say. While I appreciate the simplicity of this approach, I also agree with @DusanLesan and @laktak that this is kind of an alien concept that we don't see anywhere else. The closest thing I can think of is the shift clicking to select all files in between in regular gui file managers. Anchoring approach you mention in the linked issue sounds like the keyboard adaptation of that behavior. Since I can't really ask you implement it, we shall better leave that work for later. invert-below seems to get the job done and it is already implemented. As a middle ground, how about we merge this patch but without the default keybinding? It can serve users seeking such behavior while also avoiding confusion with other users.

This command inverts the selection for the current file and all the files below
it. Using this command twice substitutes for having a "visual" mode. This is
described in more detail in the docs and in
gokcehan#477 (comment).

Fixes gokcehan#477. While that's not exactly what was asked for, I hope it's a suitable
replacement and seems more in the spirit of `lf`, at least until a better
replacement is implemented.
@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 25, 2023

OK, I removed the binding. I also mentioned that the command is experimental in the docs. Thanks for the review!

@gokcehan gokcehan merged commit b02fd42 into gokcehan:master Feb 25, 2023
@ilyagr ilyagr deleted the invert-below branch February 25, 2023 22:19
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.

Add select + motion or visual select
4 participants