[vim/vim] Add ModeChanged autocmd (#8856)
Magnus Groß
- Triggers after every mode change (PreModeChanged could be added later if someone needs it)
- The pattern can match
old_mode:new_mode, for exampleau ModeChanged v:Vis only triggered on Visual -> Visual Line mode change andau ModeChanged *:nis triggered if any other mode switches back to normal mode.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/8856
Commit Summary
- Add ModeChanged autocmd
File Changes
- M runtime/doc/autocmd.txt (11)
- M runtime/doc/todo.txt (5)
- M runtime/syntax/vim.vim (2)
- M src/autocmd.c (7)
- M src/edit.c (3)
- M src/ex_docmd.c (1)
- M src/ex_getln.c (2)
- M src/globals.h (3)
- M src/misc1.c (38)
- M src/normal.c (3)
- M src/proto/misc1.pro (1)
- M src/testdir/test_edit.vim (20)
- M src/vim.h (3)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
Magnus Groß
Somehow very unrelated, the test_debugger.vim unit suddenly fails with this patch in Test_debug_backtrace_level():
command line..script /tmp/vim/src/testdir/runtest.vim[486]..function RunTheTest[44]..Test_debug_backtrace_level[79]..RunDbgCmd[9]..CheckDbgOutput[6]..WaitForAssert[2]..<SNR>5_WaitForCommon[11]..<lambda>26 line 1: Pattern '\\V->1 script /tmp/vim/src/testdir/Xtest1.vim[11]' does not match ' 1 script /tmp/vim/src/testdir/Xtest1.vim[11]'
I have no idea why this test is suddenly failing in very unrelated parts of the code:
call RunDbgCmd(buf, 'backtrace', [ \ '\V>backtrace', \ '\V 3 command line', \ '\V 2 script ' .. file1 .. '[11]', \ '\V->1 function <SNR>\.\*_File1Func[4]', \ '\V 0 script ' .. file2, \ '\Vline 3: func s:File2Func( arg )', \ ], \ #{ match: 'pattern' } ) " FIXME: Unexpected. Should see the a: and l: dicts from File1Func call RunDbgCmd(buf, 'echo a:arg', [ 'E121: Undefined variable: a:arg' ] ) call RunDbgCmd(buf, \ 'echo l:local_var', \ [ 'E121: Undefined variable: l:local_var' ] ) call RunDbgCmd(buf, 'up') call RunDbgCmd(buf, 'backtrace', [ \ '\V>backtrace', \ '\V 3 command line', \ '\V->2 script ' .. file1 .. '[11]', \ '\V 1 function <SNR>\.\*_File1Func[4]', \ '\V 0 script ' .. file2, \ '\Vline 3: func s:File2Func( arg )', \ ], \ #{ match: 'pattern' } )
It fails in the last part of the above block, after having gone up the backtrace. The backtrace still points to the last frame - not the correct frame. I don't know why, but when I return before (void)dict_add_string(v_event, "new_mode", rettv.vval.v_string); in the new trigger_modechanged() method, the test passes again. If I return the line after that, it fails again.
I don't have any clue at the moment why this happens, but attaching the debugger shows that somehow vim is waiting for input during the debug-unit test. Any ideas what I am doing wrong in the patch? @brammool
codecov[bot]
Codecov Report
Merging #8856 (b2a472e) into master (6b9efdd) will decrease coverage by
87.57%.
The diff coverage is0.00%.
@@ Coverage Diff @@ ## master #8856 +/- ## =========================================== - Coverage 90.03% 2.45% -87.58% =========================================== Files 151 149 -2 Lines 167776 165639 -2137 =========================================== - Hits 151059 4072 -146987 - Misses 16717 161567 +144850
| Flag | Coverage Δ | |
|---|---|---|
| huge-gcc-none | ? |
|
| huge-gcc-testgui | ? |
|
| huge-gcc-unittests | 2.45% <0.00%> (-0.01%) |
⬇️ |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/autocmd.c | 2.06% <0.00%> (-90.40%) |
⬇️ |
| src/edit.c | 0.00% <0.00%> (-92.66%) |
⬇️ |
| src/ex_docmd.c | 0.00% <0.00%> (-95.91%) |
⬇️ |
| src/ex_getln.c | 1.21% <0.00%> (-90.28%) |
⬇️ |
| src/misc1.c | 13.57% <0.00%> (-75.15%) |
⬇️ |
| src/normal.c | 0.50% <0.00%> (-95.27%) |
⬇️ |
| src/float.c | 0.00% <0.00%> (-99.22%) |
⬇️ |
| src/gui_gtk_f.c | 0.00% <0.00%> (-97.43%) |
⬇️ |
| src/crypt_zip.c | 0.00% <0.00%> (-97.06%) |
⬇️ |
| src/sha256.c | 0.00% <0.00%> (-96.94%) |
⬇️ |
| ... and 143 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 6b9efdd...b2a472e. Read the comment docs.
Magnus Groß
@vimpostor pushed 1 commit.
- 6d2700a Add ModeChanged autocmd
—
You are receiving this because you are subscribed to this thread.
Magnus Groß
Tests should pass now with the just-pushed changes. Somehow do_debug() messes up the modechange command, so I explicitly exclude the debug callee in getcmdline_int()
Magnus Groß
@vimpostor pushed 1 commit.
- ac0d7c4 Add ModeChanged autocmd
—
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
A few remarks.
The test does not check the values of "old_mode" and "new_mode"
It seems the operations in trigger_modechanged() add some overhead, while in most cases there is no ModeChanged autocommand. You can use a function similar to has_cursormoved() to bail out early.
The help could be a bit more explicit about the "old_mode" value. It should be the value of new_mode the last time the event was triggered, right?
It would be good to add an example of something useful to do with this event.
Magnus Groß
@vimpostor pushed 1 commit.
- 7b0891e Add ModeChanged autocmd
—
You are receiving this because you are subscribed to this thread.
Magnus Groß
I have implemented all of the above remarks.
My usecase for this autocmd is actually to just finally fix an open issue in my external statusline plugin: vimpostor/vim-tpipeline#11
However I think this usecase is way too niche and specific to add to help, so I instead added a usage example that I read from someone else who wanted to turn on relative linenumbers when in visual mode. It is short and concise too.
Magnus Groß
@vimpostor pushed 1 commit.
- 7975f51 Add ModeChanged autocmd
—
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
Thanks. I think it's OK now, except for the failure with MSVC - declarations must be at the start of the block.
Note that triggering an autocommand can have many side effects, and the place it's triggered from may not be prepared for it. We will need to watch out for anything that breaks and perhaps restrict what the ModeChanged event is allowed to do.
lacygoill
Could the event be fired when switching from Terminal-Normal mode to Terminal-Job mode (and vice versa)?
Magnus Groß
Could the event be fired when switching from Terminal-Normal mode to Terminal-Job mode (and vice versa)?
Currently mode() does not know the difference between Terminal-Normal and Terminal-Job, it always returns 't'. One would first have to implement a minor mode mapping in mode() before one could trigger ModeChanged for this minor mode switch.
Bram Moolenaar
> > Could the event be fired when switching from Terminal-Normal mode to
> > Terminal-Job mode (and vice versa)?
>
> Currently `mode()` does not know the difference between
> Terminal-Normal and Terminal-Job, it always returns 't'. One would
> first have to implement a minor mode mapping in `mode()` before one
> could trigger ModeChanged for this minor mode switch.
Thus it should be a matter of checking the term->tl_normal_mode flag.
That is probably only in set_terminal_mode(). But perhaps checking in
term_enter_normal_mode() works better (avoiding side effects by doing it
later).
--
If Pacman had affected us as kids we'd be running around in dark rooms,
munching pills and listening to repetitive music.
-- Marcus Brigstocke
/// 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 ///
Magnus Groß
But what if Terminal-Normal mode switches back to Normal mode? Then that would fire a "n"->"n" modechange event, which seems kinda weird.
Maybe we should add a minor mode to Terminal-Normal mode, perhaps "nt" or something like that.
Bram Moolenaar
> But what if Terminal-Normal mode switches back to Normal mode? Then
> that would fire a "n"->"n" modechange event, which seems kinda weird.
> Maybe we should add a minor mode to Terminal-Normal mode, perhaps "nt"
> or something like that.
between them. Well, quit the job, but you would do that from
Terminal-Job mode. From Normal mode to Terminal-Normal doesn't seem
possible. Does changing window count?
It's true that Normal mode and Terminal-Normal mode differ, especially
in that commands like "a" and "i" do something very different. That
would justify using "nt" for Terminal-Normal mode.
--
In many of the more relaxed civilizations on the Outer Eastern Rim of the
Galaxy, "The Hitchhiker's Guide to the Galaxy" has already supplanted the
great "Encyclopedia Galactica" as the standard repository of all knowledge
and wisdom, for though it has many omissions and contains much that is
apocryphal, or at least wildly inaccurate, it scores over the older, more
pedestrian work in two important respects.
First, it is slightly cheaper; and second, it has the words "DON'T PANIC"
inscribed in large friendly letters on its cover.
-- Douglas Adams, "The Hitchhiker's Guide to the Galaxy"
/// 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 ///
—
Magnus Groß
But what if Terminal-Normal mode switches back to Normal mode? Then
that would fire a "n"->"n" modechange event, which seems kinda weird.
Maybe we should add a minor mode to Terminal-Normal mode, perhaps "nt"
or something like that.Terminal-Normal mode is Normal mode. I don't see how you can switch
between them. Well, quit the job, but you would do that from
Terminal-Job mode. From Normal mode to Terminal-Normal doesn't seem
possible. Does changing window count?It's true that Normal mode and Terminal-Normal mode differ, especially
in that commands like "a" and "i" do something very different. That
would justify using "nt" for Terminal-Normal mode.
For me it seems like not ever firing a "n"->"nt" event is a more sane design,
than firing a "n"->"n" event.
I agree though that both designs are kinda weird. But in the end it would be
nice if ModeChanged would fire for every mode-change, so it would make sense
to implement one of those designs.
Imho the best design would be if job() would report terminal-normal mode as
"tn" (rather than "n" or "nt"), but that would be a breaking change in Vim
API.
It makes much more sense for terminal-normal mode to have "terminal" mode be
the major mode and "normal" be the minor mode.
Magnus Groß
@brammool I just realized I accidentaly fixed the returned mode length to 1 by passing VAR_UNKNOWN to f_mode().
I originally passed VAR_UNKNOWN to make the vim9script check pass in f_mode():
if (in_vim9script() && check_for_opt_bool_arg(argvars, 0) == FAIL) return;
But I also fixed the mode length to 1 with this, as the minor mode is cleared later on in f_mode() with this.
Should I fix it to be able to return minor modes? I can make the patch for this. Or is it better to only trigger this on major mode changes anyway?
zeertzjq
I just found two problems:
- To also include minor mode changes,
trigger_modechangedhas to be called much more frequently. E.g. fromntono,ixtoic, etc.. - Currently
ModeChangedis not triggered when switching between Visual mode and Select mode usingCTRL-G.
Bram Moolenaar
I was looking at how to do this, and thought I might as well implement it, instead of writing down what to change.
See patches 8.2.3461 and 8.2.3462.
What is missing is to trigger the mode change when the minor mode changes. Please create a new PR for this.
Also, I think we should skip triggering the event when old_mode and new_mode are the same.
It just doesn't seem to make sense to trigger ModeChange when mode() still returns the same value.
Bram Moolenaar
Also, it might be good to have a negation in the pattern. The example in the help is:
:autocmd ModeChanged *:v set relativenumber
So how to switch it off again? We could use ^ for this:
:autocmd ModeChanged v:^v set norelativenumber
To also catch Visual line mode the pattern "*:V" would need to be added:
:autocmd ModeChanged *:V set relativenumber
:autocmd ModeChanged V:^V set norelativenumber
And then CTRL-V for visual block mode. Perhaps we can use some character for "any Visual mode" to simplify this.
zeertzjq
Does stuff like [vV] and [^vV] work here?
zeertzjq
autocmd ModeChanged [^vV�]*:[vV�]* set relativenumber autocmd ModeChanged [vV�]*:[^vV�]* set relativenumber
zeertzjq
autocmd ModeChanged [^vV�]*:[vV�]* set relativenumber
autocmd ModeChanged [vV�]*:[^vV�]* set norelativenumber
That unknown character is CTRL-V.
Bram Moolenaar
Ah, yes, the matching happens after concatenating old_mode, ':' and new_mode.
This matches like a file name pattern, thus [] and [^] things work.
Let me add a test for that.
zeertzjq
autocmd ModeChanged [^vV�sS�]*:[vV�sS�]* set relativenumber autocmd ModeChanged [vV�sS�]*:[^vV�sS�]* set norelativenumber
For that particular purpose, maybe it is better to also include Select mode. The unknown character that comes before closing square brackets is CTRL-S.
zeertzjq
I think the addition of mode nt causes some problems with ModeChanged. Try this:
- Add autocommand
:autocmd ModeChanged nt:i echomsg 'This should not happen' - Open a new terminal with
:term - Press
<C-\><C-N>to go to Terminal-Normal mode - Press
vto go to Visual mode - Press
<Esc>to go back to Terminal-Normal mode - Press
<C-W><C-W>to go to another split - Press
ito go to Insert mode - Press
<Esc>to go back to Normal mode - View
:messagesandThis should not happneappears
Apparently in step 7 last_mode is nt (which is wrong), while new_mode is i
Bram Moolenaar
> I think the addition of mode `nt` causes some problems with `ModeChanged`. Try this:
>
> 1. Add autocommand `:autocmd ModeChanged nt:i echomsg 'This should not happen'`
> 2. Open a new terminal with `:term`
> 3. Press `<C-\><C-N>` to go to Terminal-Normal mode
> 4. Press `v` to go to Visual mode
> 5. Press `<Esc>` to go back to Terminal-Normal mode
> 6. Press `<C-W><C-W>` to go to another split
> 7. Press `i` to go to Insert mode
> 8. Press `<Esc>` to go back to Normal mode
> 9. View `:messages` and `This should not happne` appears
>
> Apparently in step 7 `last_mode` is `nt` (which is wrong), while
It appears the switching to another window should have triggered
ModeChanged, going from "nt" to "n".
However, the autocommand may want to do something different when in
another window. The example of setting 'relativenumber' for some mode
and switching it off again when leaving that mode will not work
properly.
I'm not sure what is the best way to do this. We could make the buffer
number available that is related to old_mode and new_mode perhaps. But
then the user still has to check it.
Another way would be to keep the old mode per window. Then switching
window won't do anything, and when entering insert mode old_mode would
be "n", the previous mode in that window. This might work best. The
user can also check the current buffer or window if needed.
This should work then:
au ModeChanged *:[vV<C-V>]* set relativenumber
au ModeChanged [vV<C-V>]*:[^vV<C-V>]* set norelativenumber
--
From "know your smileys":
<>:-) Bishop
/// 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 ///
—
zeertzjq
wincmd can switch window without changing the current mode, so remembering last_mode per window indeed seem better. Maybe ModeChanged can be triggered when entering a window if and only if the current mode is different from last_mode for that window.
zeertzjq
Another thing I just randomly thought of: should last_mode be initialized to i instead of n when 'insertmode' is set?
Bram Moolenaar
> Another thing I just randomly thought of: should `last_mode` be
> initialized to `i` instead of `n` when `'insertmode'` is set?
when the generated insert command is executed.
--
From "know your smileys":
o _ _ _
_o /\_ _ \\o (_)\__/o (_)
_< \_ _>(_) (_)/<_ \_| \ _|/' \/
(_)>(_) (_) (_) (_) (_)' _\o_
/// 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 ///
—
zeertzjq
With bufhidden=hide it is possible to switch between a Terminal buffer and a non-Terminal buffer in the same window, so maybe ModeChanged has to be triggered in that case. (E.g. update statusline)
zeertzjq
It seems ModeChanged is not called if do_pending_operator ends Visual mode.
zeertzjq
It seems
ModeChangedis not triggered ifdo_pending_operatorends Visual mode.
Also not triggered when using mouse to start or end Visual mode.
zeertzjq
Something else that may be related: switching to a window that has the same buffer as the current window using <C-W><C-W> or a mapping to <Cmd>wincmd w<CR> doesn't end Visual mode, but switching to a window that has a different buffer does. Things are starting to seem more complicated...
zeertzjq
On the other hand, if last_mode is global, I think this can be used to deal with the window switching problem of the example:
autocmd WinEnter * let &relativenumber = mode() =~# '[vV�]'
Bram Moolenaar
Are you going to make a PR to fix the problems you noticed?
zeertzjq
IDK, I'm thinking about it, but I may not have enough time.
Magnus Groß
IDK, I'm thinking about it, but I may not have enough time.
I can do it too, since I brought this autocmd here and should probably fix it then.
However, I think the API design is not yet clear to me. You talked about making ModeChanged buffer-local, but I strongly disagree with this design. A user of this autocmd is interested in every case where the mode changes, including mode changes that are due to switching to another window. So why not keep it simple and keep ModeChanged global and just trigger a ModeChanged when switching to another window.
I agree with Bram, that inside trigger_modechanged() there should be an early return, if new mode and old mode are the same. That way all places where this function is called, can avoid unnecessary branching and I think this approach is much more clean, than making ModeChanged buffer-local and having to sort out all edge cases.
zeertzjq
Did I talk about making ModeChanged buffer-local?
zeertzjq
One thing I said is that changing the buffer of the current window can change mode as well, as a Terminal buffer has different modes from a non-Terminal buffer.
Magnus Groß
Did I talk about making
ModeChangedbuffer-local?
Sorry, I meant window-local. Bram proposed it as best solution here. But it was still up for discussion (I think).
zeertzjq
- If
last_modeis global, it is always the last mode anywhere in Vim, and it is still possible to remember the last mode in each window usingWinLeave. - If
last_modeis window-local, it is the last mode in each window, and it is hard to determine when it was saved, so it is hard to achieve what a globallast_modecan achieve
So maybe a global last_mode has an advantage over a window-local last_mode.
Bram Moolenaar
Please discuss the alternatives using examples. One example was to set 'relativenumber' when in Visual mode, and reset it when leaving Visual mode. When using CTRL-W CTRL-W to jump to another window, which has another buffer, Visual mode is ended. ModeChange must then be triggered in the original window for this to work.
If the other window shows the same buffer, then 'relativenumber' would need to be set there too, even though mode does not actually change. I'm not sure how best to handle this. It might require the autocommand checking the window ID, which makes this more complicated.
Magnus Groß
When using CTRL-W CTRL-W to jump to another window, which has another buffer, Visual mode is ended. ModeChange must then be triggered in the original window for this to work.
Like @zeertzjq said, these problems can be fixed with WinEnter and WinLeave, for example this particular example would be fixed with: autocmd WinLeave * let &relativenumber = mode() =~# '[vV�]'
I think that a buffer-local ModeChanged would be rather confusing, as for example switching windows can obviously be an user-visible modechange, but would then not trigger a modechange. I am open for being convinced though if you truly believe that buffer-local is the best way to go.
zeertzjq
Oh, I just realized there needs to be a ^ to match the start of the pattern (^[vV�] instead of [vV�]). Otherwise it will also match Rv.
Bram Moolenaar
> > When using CTRL-W CTRL-W to jump to another window, which has
> > another buffer, Visual mode is ended. ModeChange must then be
> > triggered in the original window for this to work.
>
> Like @zeertzjq said, these problems can be fixed with `WinEnter` and
> `autocmd WinLeave * let &relativenumber = mode() =~# '[vV�]'`
Hmm, so we could do it with four autocommands:
au ModeChanged [vV<C-V]*:* let &l:relativenumber = mode() =~# '^[vV�]'
au ModeChanged *:[vV<C-V]* let &l:relativenumber = mode() =~# '^[vV�]'
au WinEnter * let &l:relativenumber = mode() =~# '^[vV�]'
au WinLeave * let &l:relativenumber = mode() =~# '^[vV�]'
> I think that a buffer-local ModeChanged would be rather confusing, as
> for example switching windows can obviously be an user-visible
> modechange, but would then not trigger a modechange. I am open for
> being convinced though if you truly believe that buffer-local is the
> best way to go.
it just works, but perhaps it's not useful.
--
In a world without walls and borders, who needs windows and gates?
/// 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 ///
—
lacygoill
I'm working on some code to make gv remember more than just the last visual area. When quitting visual mode to normal mode, I need to save the positions of the visual marks. Unfortunately, the latter are not set when ModeChanged is fired. As a result, I need to save them via a timer, which looks awkward.
Could we delay ModeChanged a little (until the visual marks are set)?
diff --git a/src/normal.c b/src/normal.c index eafd1fddb..6cef7dd61 100644 --- a/src/normal.c +++ b/src/normal.c @@ -1386,7 +1386,6 @@ end_visual_mode_keep_button() #endif VIsual_active = FALSE; - trigger_modechanged(); setmouse(); mouse_dragging = 0; @@ -1402,6 +1401,7 @@ end_visual_mode_keep_button() curwin->w_cursor.coladd = 0; may_clear_cmdline(); + trigger_modechanged(); adjust_cursor_eol(); }
Magnus Groß
Could we delay
ModeChangeda little (until the visual marks are set)?
I think this does make sense. I have included your diff in #8999
Bram Moolenaar
> I'm working on some code to make `gv` remember more than just the last
> visual area. When quitting visual mode to normal mode, I need to save
> the positions of the visual marks. Unfortunately, the latter are not
> set when `ModeChanged` is fired. As a result, I need to save them via
> a timer, which looks awkward.
>
> ```diff
> diff --git a/src/normal.c b/src/normal.c
> index eafd1fddb..6cef7dd61 100644
> --- a/src/normal.c
> +++ b/src/normal.c
> @@ -1386,7 +1386,6 @@ end_visual_mode_keep_button()
> #endif
>
> VIsual_active = FALSE;
> - trigger_modechanged();
> setmouse();
> mouse_dragging = 0;
>
> @@ -1402,6 +1401,7 @@ end_visual_mode_keep_button()
> curwin->w_cursor.coladd = 0;
> may_clear_cmdline();
>
> + trigger_modechanged();
> adjust_cursor_eol();
> }
in an invalid position.
--
A bad peace is better than a good war. - Yiddish Proverb
/// 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 ///
—
Magnus Groß
>Maybe it's better after adjust_cursor_eol(), so that the cursor is not
>in an invalid position.