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: support unyank #313

Merged
merged 2 commits into from
Jan 31, 2024
Merged

feat: support unyank #313

merged 2 commits into from
Jan 31, 2024

Conversation

15cm
Copy link
Contributor

@15cm 15cm commented Oct 24, 2023

There are two options to put the unyank ability:

  • Option 1: Register a new command like in this PR
  • Option 2: Add the unyank to the escape sequence, maybe after the escape_select.

I chose option 1 because the yank function is its own module but the escape sequence belongs to the tab class. Option 2 is infeasible without touching the module structure.

I just randomly choose a meaninful and unoccupied key binding for the unyank function. Feel free to modify or remove it from the preset. I personally remap the yank related functions to dance keys such as ["y" "y"].

@sxyazi
Copy link
Owner

sxyazi commented Oct 24, 2023

When do you need to "unyank"? People typically don't unregister a file once they've placed it in the register IMO.

@15cm
Copy link
Contributor Author

15cm commented Oct 24, 2023

They do when they register the files by mistake, from two aspects:

  1. I yank some files by mistake, the indicator of the yank never goes away because the yank indicator sticks with the yanked files. Even unselect doesn't help.
  2. I yank some files by mistake and simply want to remove them from the register (and my head) that I have yanked something. So I don't need to worry about pressing the paste key bindings by mistake when continue browsing the files.

A video to demonstrate that the yank indicator get on the way of the select indicator https://github.com/sxyazi/yazi/assets/7759556/d0065a22-154f-4bcb-9244-77af3f1e82cf. What I did:

  • Select the first 4 files
  • Yank them.
  • Oh I make a mistake. I only want to select the first 3 files. But with the yank indicator I don't know what I selected this time.
  • Ok I unyank to remove the indicator and clear the register.
  • I can start from scratch now.

@sxyazi
Copy link
Owner

sxyazi commented Oct 24, 2023

I think this is an issue that we can optimize from a UI perspective, "unyank" seems to be not so easy to use.

I've tried creating another markers component, aiming to coexist with the yank in the selection state, while toning down the yank (from bg to fg). Maybe we can tweak the colors further anyway, any thoughts?

1.mp4

@15cm
Copy link
Contributor Author

15cm commented Oct 25, 2023

The UI improvement is another fundamental fix of this issue and worth another ticket. It addresses the problem above when "Oh I make a mistake. I only want to select the first 3 files. But with the yank indicator I don't know what I selected this time.".

However, I still find the unyank support necessary because it addresses the other aspect of this issue: unlike other file managers (ranger in TUI, nautils in GUI), Yazi provides a visualization of your current register. It's a decent feature. But meanwhile it means users are always aware of what's in their register even if they don't intend to. An unyank operation will help this scenario: a user yanks a file but later decides not to copy that file and any other files. I didn't find a way to get rid of the yank indicator at this point and thus requested an unyank operation. Some FMs don't have this issue because they don't have the indicator in the first place.

@15cm
Copy link
Contributor Author

15cm commented Oct 25, 2023

Meanwhile I thought a bit about option 2 in my first comment #313 (comment), i.e. let escape handling the unyank with a priority behind unselect. I don't think we should go that direction because users can accidentally hit more than they want. I don't think we should bind more things to ESC in the manager unless it's really intuitive.

@sxyazi
Copy link
Owner

sxyazi commented Oct 26, 2023

I'll wait for a while on #319

@sxyazi sxyazi mentioned this pull request Nov 11, 2023
55 tasks
@15cm
Copy link
Contributor Author

15cm commented Dec 4, 2023

There hasn't been any progress on #319. Shall we consider merging the PR to mitigate the indicator issue for now? If that RFC gets implemented, the unyank and yank functionality will be deprecated together.

@sxyazi
Copy link
Owner

sxyazi commented Dec 4, 2023

Ah sorry, I've been a bit strapped for time lately. I'll follow up on this RFC once the plugin system is completed.

As it stands, there's a good chance this RFC will be implemented - though it still lacks some specific implementation details, so I'll tread carefully on merging this PR - I'd like to avoid the awkward situation of "introducing a new behavior and quickly breaking it".

@fritzrehde
Copy link

I would like to just throw in my 2 cents as well: I don't think this is an issue that can be fixed by a UI tweak (like @sxyazi proposed a while back). After finishing a copy operation, I would like to somehow get back to a state where nothing is in the copy "buffer". However, the current implementation (without this an unyank) means that this copy "buffer" still exists and cannot be emptied/removed after a copy operation, which is slightly inconvenient if I were to repeatadly invoke the paste command. So, I think it would be great to leave the choice between these two behaviours to the user:

  1. Once files are copied once, one can call paste repeatedly again (this is the current behaviour).
  2. After files have been copied, the user can decide (maybe by mapping an unyank operation to <Esc> or similar) to remove the copy "buffer", which means that subsequent paste invocations do nothing.

I am not sure which of these two should then be the default, but allowing the user the freedom to choose between the two would be really cool! Therefore, I am in favour of adding an unyank command.

There are two options to put the unyank ability:
- Option 1: Register a new command like in this CL
- Option 2: Add the unyank to the escape sequence, maybe after the escape_select.

I chose option 1 because the yank function is its own module but the escape
sequence belongs to the tab class. Option 2 is infeasible without touching the
module structure.

I just randomly choose a meaninful and unoccuped key binding for the `unyank`
function. Feel free to modify or remove it from the preset. I personally remap
the yank related functions to dance keys such as ["y" "y"].
@sxyazi
Copy link
Owner

sxyazi commented Jan 31, 2024

Thank you, merged!

@sxyazi sxyazi changed the title feat: Support unyank feat: support unyank Jan 31, 2024
@sxyazi sxyazi merged commit b013dff into sxyazi:main Jan 31, 2024
3 checks passed
@15cm 15cm deleted the unyank branch February 1, 2024 05:57
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants