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

Fix old values shown in select_register #5242

Closed

Conversation

xJonathanLEI
Copy link
Contributor

Fixes #5241.

@kirawi kirawi added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Dec 24, 2022
@kirawi kirawi added the C-bug Category: This is a bug label Jan 1, 2023
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fix the display for registers that are being used like a stack but this doesn't makes sense for other registers. Kakoune's help for " shows the docs for each register and special registers which is more helpful IMO.

If I remember correctly, the current behavior matches https://github.com/Delapouite/kakoune-registers/blob/master/registers.kak

@xJonathanLEI
Copy link
Contributor Author

Hmm I wasn't aware of any register with a len() larger than 1 (for this change to make a difference to them) but not used like a stack at the same time. I made this change because it's showing wrong value for the search (/) register... it shows the oldest value instead of the latest.

How would you like the issue to be fixed if the proposed changes are not desired? Thanks in advance.

@the-mikedavis
Copy link
Member

You can yank multiple selections into a register.

The current display is not really wrong: it's what you will get when you paste that register (with "/p). It's just that search uses the last element instead of the first.

I think it could make sense to reverse the direction of the stack so that pushing operations push to the front and we always use the front element in search. This would diverge from Kakoune and we would probably want to change the datastructure used for storing values within the Register though.

@xJonathanLEI
Copy link
Contributor Author

I think it could make sense to reverse the direction of the stack so that pushing operations push to the front and we always use the front element in search.

Ah I agree this makes the most sense.

change the datastructure used for storing values within the Register

Yeah inserting at the beginning of Vec would be extremely inefficient.

@ohno418
Copy link
Contributor

ohno418 commented Mar 26, 2023

Hi. I'm a bit interested in this, so took a look into how registers work.

@the-mikedavis

The current display is not really wrong: it's what you will get when you paste that register (with "/p). It's just that search uses the last element instead of the first.

Given that the / register is supposed to have a last search value (https://docs.helix-editor.com/master/usage.html#special-registers), it feels weird that "/p pastes the first search value. (Not only the case for the / register, as well as the : register.)

So my question is why does the / register have multiple values like logs? If I understand correctly, in the case of the yank register, we just rewrite to the new value, instead of pushing it (code here). Why the / register doesn't do the same as that?

(I came from NeoVim and not Kakoune. Sorry if I incorrectly understand registers or selections.)

@the-mikedavis
Copy link
Member

I haven't looked through the history but I think that note in the docs is just out of date. The search commands push new searches to the end of the / register like a stack. The prior values in the stack are used for history completion in the prompt UI element.

The yank commands use registers differently: each range in a selection has its contents saved as an element in the register.

@ohno418
Copy link
Contributor

ohno418 commented Mar 27, 2023

The prior values in the stack are used for history completion in the prompt UI element.

Ah, got it.

The search commands push new searches to the end of the / register like a stack.

The yank commands use registers differently: each range in a selection has its contents saved as an element in the register.

Yeah. Hmm, so I'm not really sure, but it looks that the problem here is the inconsistency. Some registers (like the search register) have their values as history stacks, and others (like the yank register) have them simply to refer to multiple selections.

So, changing the data structures. That makes sense. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

select_register incorrectly shows the oldest value in registers
4 participants