keybind not working: delete_selection_noyank, search_prev

[timokoesters]

Summary

This should do the same as ?=<enter>. This works if I replace delete_selection_noyank with delete_selection.

"C-l" = [
  ":append-output echo -n =",
  "search_selection",
  "delete_selection_noyank",
  "search_prev",
]

The response on matrix was:

It could be the case that delete_selection_noyankkeeps the register at _ so search_prev tries looking at _ (which is the null register) so it doesn't do anything

https://matrix.to/#/!zMuVRxoqjyxyjSEBXc:matrix.org/$nt9EkBO1jiVDOp1LDJf0hBHk_MmipL6RttvCJMU1-es?via=matrix.org&via=tchncs.de&via=mozilla.org

Reproduction Steps

No response

Helix log

No response

Platform

Linux

Terminal Emulator

Alacritty

Installation Method

AUR

Helix Version

helix 23.10

[the-mikedavis]

The _noyank commands set the register to _ but the register isn't reset if the commands are run in a keymap sequence. These functions https://github.com/helix-editor/helix/blob/0da58656958184677b44372f72a922baacfc4940/helix-term/src/commands.rs#L2698-L2710

could be updated to save the value of cx.register before cx.register = Some('_') and reset cx.register to that value after the deletion/change

EDIT: or instead, pass a boolean for wether to yank or not. Passing in the _ register is not much different than a boolean and is just less explicit

[voiceroy]

Something along these lines?

fn delete_selection_noyank(cx: &mut Context) {
    let old_register = cx.register;
    cx.register = Some('_');
    delete_selection_impl(cx, Operation::Delete);
    cx.register = old_register;
}

fn change_selection(cx: &mut Context) {
    // let old_register = cx.register;
    delete_selection_impl(cx, Operation::Change);
    // cx.register = old_register;
}

fn change_selection_noyank(cx: &mut Context) {
    let old_register = cx.register;
    cx.register = Some('_');
    delete_selection_impl(cx, Operation::Change);
    cx.register = old_register;
}

[the-mikedavis]

On second thought I would prefer we introduce an extra parameter yank: bool to delete_selection_impl:

fn delete_selection(cx: &mut Context) {
    delete_selection_impl(cx, Operation::Delete, true);
}

fn delete_selection_noyank(cx: &mut Context) {
    delete_selection_impl(cx, Operation::Delete, false);
}

fn change_selection(cx: &mut Context) {
    delete_selection_impl(cx, Operation::Change, true);
}

fn change_selection_noyank(cx: &mut Context) {
    delete_selection_impl(cx, Operation::Change, false);
}

It would mean that we do an unnecessary allocation when you use "_d or "_c" but it's more explicit and simpler, and also fixes this issue.

[voiceroy]

I think

"C-l" = [
  ":append-output echo -n =",
  "search_selection",
  "delete_selection_noyank",
  "search_prev",
]

appends an = next to the current character and then selects it, then deletes it without yanking (assuming it's fixed), and then searches for a previous match which would be the = found at the initial 'run' of this keybind. Whenever the keybind C-l is run this would be the process. Hence not going to previous selection as N would do after ?=<enter>.

Eg:

Let the file contents be:

=    # line 1
test # line 2
=    # line 3

1. test (line 2, pressed C-l)
2. test= (= appended)
3. test|=| (= selected)
4. test (= deleted)
5. = (found an = while doing search_prev at line 1)
6. == (if pressed C-l again, appending =)
7. =|=| (appended = selected)
8. = (delete the selected =)
9. = (search_prev, still at line 1 and not going to the = at line 3)
10. and the whole thing from 6-9 repeats if C-l is pressed

Can anyone confirm that what I said above is correct?