[vim/vim] v_P does not overwrite unnamed registers (PR #9591)
Shougo
I have changed v_P behavior.
Current both v_p and v_P behavior overwrites unnamed register.
But it is very annoying if I want to replace more words.
vim-operator-replace is available for it.
https://github.com/kana/vim-operator-replace
But I think it should be builtin key mapping.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/9591
Commit Summary
- 0787e97 v_P does not overwrite unnamed registers
File Changes
(3 files)
- M runtime/doc/visual.txt (1)
- M src/normal.c (2)
- M src/testdir/test_visual.vim (18)
Patch Links:
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
codecov[bot]
Codecov Report
Merging #9591 (0787e97) into master (19e69a6) will decrease coverage by
2.76%.
The diff coverage is100.00%.
@@ Coverage Diff @@ ## master #9591 +/- ## ========================================== - Coverage 83.18% 80.42% -2.77% ========================================== Files 154 152 -2 Lines 174434 172914 -1520 Branches 39243 39243 ========================================== - Hits 145107 139064 -6043 - Misses 17201 21126 +3925 - Partials 12126 12724 +598
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | 81.88% <100.00%> (+<0.01%) |
⬆️ |
| huge-gcc-none | ? |
|
| huge-gcc-unittests | 2.03% <0.00%> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/normal.c | 86.99% <100.00%> (-3.19%) |
⬇️ |
| src/libvterm/src/rect.h | 0.00% <0.00%> (-96.78%) |
⬇️ |
| src/libvterm/src/state.c | 34.80% <0.00%> (-54.37%) |
⬇️ |
| src/libvterm/src/keyboard.c | 36.84% <0.00%> (-50.79%) |
⬇️ |
| src/libvterm/include/vterm.h | 0.00% <0.00%> (-44.45%) |
⬇️ |
| src/libvterm/src/parser.c | 55.41% <0.00%> (-40.47%) |
⬇️ |
| src/libvterm/src/pen.c | 44.37% <0.00%> (-39.33%) |
⬇️ |
| src/libvterm/src/encoding.c | 37.37% <0.00%> (-36.16%) |
⬇️ |
| src/libvterm/src/vterm.c | 39.17% <0.00%> (-27.21%) |
⬇️ |
| src/if_perl.xs | 54.42% <0.00%> (-22.90%) |
⬇️ |
| ... and 130 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update 19e69a6...0787e97. Read the comment docs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
> I have changed `v_P` behavior.
> Current both `v_p` and `v_P` behavior overwrites unnamed register.
> But it is very annoying if I want to replace more words.
>
> `vim-operator-replace` is available for it.
> https://github.com/kana/vim-operator-replace
> But I think it should be builtin key mapping.
Currently both "p" and "P" do the same thing, this is also documented.
I would think it unlikely that users currently use "P", since it's
slightly more work to type. Also, most times users would not depend on
the register contents. Thus only very few users would be affected.
Anyone else has an opinion?
--
An indication you must be a manager:
You feel sorry for Dilbert's boss.
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
zeertzjq
An alternative implementation may be to use CTRL-R to specify which register to overwrite, like #9531. Then one can map P in Visual mode to something like <C-R>_p. This is more backward-compatible, but may require a bit more work.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
I think it's not uncommon to want to keep the register content. So there are two ways: Swap the selected text with the register (so you can put it somewhere else), or keep the register, so you can repeat the same change in another place. Having to type CTRL-R _ seems a bit much. But if we do want to maintain backwards compatibility that would be the way to do it.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
matveyt
If I understand correctly, the patch simply deletes the selection into "blackhole" register. It'd be more useful, IMO, if it'd delete it into "minus" or "one" (like v_p currently does) and only bypass changing "unnamed" pointer (or restore it before return).
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
If I understand correctly, the patch simply deletes the selection into "blackhole" register. It'd be more useful, IMO, if it'd delete it into "minus" or "one" (like v_p currently does) and only bypass changing "unnamed" pointer (or restore it before return).
It is possible to change. But unnamed register is broken if "minus" or "one" register is specified in the source code.
So unnamed register must be restored. The feature is really useful?
I like the simple patch.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
And the restore does not work for me...
diff --git a/src/normal.c b/src/normal.c index fb14a9fad..a1c5e815f 100644 --- a/src/normal.c +++ b/src/normal.c @@ -7540,6 +7540,7 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) if (VIsual_active) { + void *reg3 = NULL; // Putting in Visual mode: The put text replaces the selected // text. First delete the selected text, then put the new text. // Need to save and restore the registers that the delete @@ -7562,8 +7563,11 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) reg1 = get_register(regname, TRUE); } + if (cap->cmdchar == 'P') + reg3 = get_register(0, TRUE); + // Now delete the selected text. Avoid messages here. - cap->oap->regname = cap->cmdchar == 'P' ? '_' : NUL; + cap->oap->regname = cap->cmdchar == 'P' ? '-' : NUL; cap->cmdchar = 'd'; cap->nchar = NUL; ++msg_silent; @@ -7583,6 +7587,10 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) put_register(regname, reg1); } + // Unnamed register must be restored + if (reg3 != NULL) + put_register(0, reg3); + // When deleted a linewise Visual area, put the register as // lines to avoid it joined with the next line. When deletion was // characterwise, split a line when putting lines.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
matveyt
@Shougo Unnamed register is simply a pointer, so we don't need to preserve the contents. AFAIU, the code could be trivially implemented with the help of get_y_current(), set_y_current(), get_y_previous(), set_y_previous() and a few #if defined(FEAT_XXX). All the ingredients are listed at the top of src/register.c.
Concerning usefulness, I take it rather as a consistency. Whenever normal-mode command deletes some text it should be saved into a register.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
Unnamed register is simply a pointer, so we don't need to preserve the contents. AFAIU, the code could be trivially implemented with the help of get_y_current(), set_y_current(), get_y_previous(), set_y_previous() and a few #if defined(FEAT_XXX). All the ingredients are listed at the top of src/register.c.
I have tested it and it does not work.
/* * Registers: * 0 = unnamed register, for normal yanks and puts * 1..9 = registers '1' to '9', for deletes * 10..35 = registers 'a' to 'z' ('A' to 'Z' for appending) * 36 = delete register '-' * 37 = Selection register '*'. Only if FEAT_CLIPBOARD defined * 38 = Clipboard register '+'. Only if FEAT_CLIPBOARD and FEAT_X11 defined */ static yankreg_T y_regs[NUM_REGISTERS];
The content must be saved....
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
diff --git a/src/normal.c b/src/normal.c
index fb14a9fad..efb8c9f62 100644
--- a/src/normal.c +++ b/src/normal.c @@ -7540,6 +7540,7 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) if (VIsual_active) {
+ yankreg_T save_unnamed = { 0 };
// Putting in Visual mode: The put text replaces the selected
// text. First delete the selected text, then put the new text.
// Need to save and restore the registers that the delete
@@ -7562,8 +7563,13 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) reg1 = get_register(regname, TRUE); } +#if defined(FEAT_CLIPBOARD) || defined(PROTO)
+ if (cap->cmdchar == 'P')
+ save_unnamed = *get_y_register(0); +#endif
+ // Now delete the selected text. Avoid messages here. - cap->oap->regname = cap->cmdchar == 'P' ? '_' : NUL; + cap->oap->regname = cap->cmdchar == 'P' ? '-' : NUL; cap->cmdchar = 'd'; cap->nchar = NUL; ++msg_silent;
@@ -7583,6 +7589,12 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) put_register(regname, reg1); } +#if defined(FEAT_CLIPBOARD) || defined(PROTO)
+ // Unnamed register must be restored
+ if (save_unnamed.y_array) + *get_y_register(0) = save_unnamed; +#endif
+
// When deleted a linewise Visual area, put the register as
// lines to avoid it joined with the next line. When deletion was
// characterwise, split a line when putting lines.It does not work. I don't know why.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
I have checked the code. get_register()/put_register() should work...
I don't know why it does not work.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
matveyt
Unnamed register is not just pointer.
I guess you're confused by the comment in source code. However, y_regs[0] is the same as :h quote0 that yank overwrites by default; while :h quotequote is really y_current pointer.
So, my proposal, as a rough sketch, is:
diff --git a/src/normal.c b/src/normal.c index b6c59d310..4422d0f1f 100644 --- a/src/normal.c +++ b/src/normal.c @@ -7501,6 +7501,8 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) void *reg1 = NULL, *reg2 = NULL; int empty = FALSE; int was_visual = FALSE; + int save_unnamed = FALSE; + yankreg_T *old_y_current, *old_y_previous; int dir; int flags = 0; @@ -7548,6 +7550,7 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) // Need to save and restore the registers that the delete // overwrites if the old contents is being put. was_visual = TRUE; + save_unnamed = cap->cmdchar == 'P'; regname = cap->oap->regname; #ifdef FEAT_CLIPBOARD adjust_clip_reg(®name); @@ -7566,6 +7569,11 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) } // Now delete the selected text. Avoid messages here. + if (save_unnamed) + { + old_y_current = get_y_current(); + old_y_previous = get_y_previous(); + } cap->cmdchar = 'd'; cap->nchar = NUL; cap->oap->regname = NUL; @@ -7574,6 +7582,11 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) do_pending_operator(cap, 0, FALSE); empty = (curbuf->b_ml.ml_flags & ML_EMPTY); --msg_silent; + if (save_unnamed) + { + set_y_current(old_y_current); + set_y_previous(old_y_previous); + } // delete PUT_LINE_BACKWARD; cap->oap->regname = regname;
You can check that :reg "0123456789- outputs the same after v_p or v_P, except unnamed register.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
Oh, thanks. It works.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
But it breaks existing tests...
Found errors in Test_blockwise_visual():
command line..script /home/shougo/src/vim/src/testdir/runtest.vim[456]..function RunTheTest[44]..Test_blockwise_visual line 15: Expected ['123start here56', '2
34start here67', '345start here78', '', 'test text test tex rt here', '\t\tsomext', '\t\ttesext'] but got ['123456', '234567', '345678', '', 'test text test te
x rt here', '\t\tsomext', '\t\ttesext']
Found errors in Test_dotregister_paste():
command line..script /home/shougo/src/vim/src/testdir/runtest.vim[456]..function RunTheTest[44]..Test_dotregister_paste line 4: Expected 'hello world world' bu
t got 'hello worldhello world'
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
matveyt
"Tiny" build fail. There should be some #if defined(FEAT_XXX) guards there (like those in src/register.c, I guess). I just forgot to add it.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
K.Takata
@k-takata commented on this pull request.
In src/normal.c:
> @@ -7568,12 +7568,14 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) reg1 = get_register(regname, TRUE); } +#if defined(FEAT_CLIPBOARD) || defined(PROTO)
defined(PROTO) is not needed inside a function.
It is needed for generating function prototypes using the cproto program.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
@Shougo commented on this pull request.
In src/normal.c:
> @@ -7568,12 +7568,14 @@ nv_put_opt(cmdarg_T *cap, int fix_indent) reg1 = get_register(regname, TRUE); } +#if defined(FEAT_CLIPBOARD) || defined(PROTO)
OK. Fixed.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Shougo
The tests are passed.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
lacygoill
Documenting the new command somewhere at :help v_P and at :help visual-index would make it more discoverable:
diff --git a/runtime/doc/change.txt b/runtime/doc/change.txt index 2799dc9e6..d1975d563 100644 --- a/runtime/doc/change.txt +++ b/runtime/doc/change.txt @@ -1176,11 +1176,9 @@ register. With blockwise selection it also depends on the size of the block and whether the corners are on an existing character. (Implementation detail: it actually works by first putting the register after the selection and then deleting the selection.) -The previously selected text is put in the unnamed register. If you want to -put the same text into a Visual selection several times you need to use -another register. E.g., yank the text to copy, Visually select the text to -replace and use "0p . You can repeat this as many times as you like, the -unnamed register will be changed each time. +With |p|, the previously selected text is put in the unnamed register, but not +with |P|. The latter is useful if you want to put the same text into a Visual +selection several times. When you use a blockwise Visual mode command and yank only a single line into a register, a paste on a visual selected area will paste that single line on diff --git a/runtime/doc/index.txt b/runtime/doc/index.txt index cbf363a77..2970449fc 100644 --- a/runtime/doc/index.txt +++ b/runtime/doc/index.txt @@ -938,6 +938,8 @@ tag command note action in Visual mode ~ |v_J| J 2 join the highlighted lines |v_K| K run 'keywordprg' on the highlighted area |v_O| O Move horizontally to other corner of area. +|v_P| P replace visual selection with register, + without changing unnamed register Q does not start Ex mode |v_R| R 2 delete the highlighted lines and start insert @@ -1000,6 +1002,7 @@ tag command note action in Visual mode ~ |v_i{| i{ same as iB |v_i}| i} same as iB |v_o| o move cursor to other corner of area +|v_p| p replace visual selection with register |v_r| r 2 replace highlighted area with a character |v_s| s 2 delete highlighted area and start insert |v_u| u 2 make highlighted area lowercase
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
Yes, I'll include that.
—
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.