[vim/vim] Add "Load All" / "Ignore All" options to file reload dialog (PR #12158)

88 views
Skip to first unread message

Yee Cheng Chin

unread,
Mar 15, 2023, 10:33:49 AM3/15/23
to vim/vim, Subscribed

Currently when one or more edited files are changed outside the editor, Vim pops up a dialog asking if the user wants to reload the file. Available options are "OK" (ignore), "Load File", and "Load File and Option". However, if there are a lot of files changed at once (e.g. if you just did a git pull), these dialog spams could become quite annoying to deal with as they are modal. You can set autoread to work around the issue but even then you can only set it after you have handled the 30 dialog press and whatnot.

This PR adds an option to reload or ignore all files in one go, to make the experience better. Because the way Vim checks mod time is by going through files one by file and display the dialog inside the loop, there isn't a way to only selectively show the "load all" option only if more than one file has been changed right now. Also, this only applies to the default check / :checktime, and :diffupdate!. Other usages of this dialog box like :checktime <buf> or :b <buf> only affect one file and will use the old dialog box to avoid confusing the user.

Note: I also changed the mnemonics of "Load File &and Options" to "Load File and O&ption". This may seem less intuitive but I think the "Load All" option is actually going to be more useful to most people and therefore rebinded "a" to "Load All" instead. I can revert this part if desired.

Localization will need to be updated with the new dialog box options.

I have not implemented specific tests for this yet. Wanted to gather feedback first before having to iterate on tests.

Also see: #9579, which implemented the "Load File and Options" feature.

Note: This is a port of MacVim's logic which added this downstream a while ago.


You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/12158

Commit Summary

  • 574fa0b Add "Load All" / "Ignore All" options to file reload dialog

File Changes

(7 files)

Patch Links:


Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158@github.com>

codecov[bot]

unread,
Mar 15, 2023, 10:46:09 AM3/15/23
to vim/vim, Subscribed

Codecov Report

Merging #12158 (574fa0b) into master (e764d1b) will increase coverage by 0.36%.
The diff coverage is 62.50%.

@@            Coverage Diff             @@
##           master   #12158      +/-   ##
==========================================
+ Coverage   81.95%   82.32%   +0.36%     
==========================================
  Files         164      154      -10     
  Lines      194086   183691   -10395     
  Branches    43828    41400    -2428     
==========================================
- Hits       159069   151223    -7846     
+ Misses      22183    19921    -2262     
+ Partials    12834    12547     -287     
Flag Coverage Δ
huge-clang-none 82.67% <74.07%> (+<0.01%) ⬆️
huge-gcc-none ?
huge-gcc-testgui 51.96% <32.25%> (-0.01%) ⬇️
huge-gcc-unittests 0.29% <0.00%> (-0.01%) ⬇️
linux 82.32% <62.50%> (-0.07%) ⬇️
mingw-x64-HUGE ?
mingw-x86-HUGE ?
windows ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/fileio.c 75.46% <52.00%> (-0.56%) ⬇️
src/buffer.c 86.68% <100.00%> (-0.88%) ⬇️
src/diff.c 83.18% <100.00%> (-0.20%) ⬇️
src/ex_cmds.c 86.12% <100.00%> (+0.12%) ⬆️
src/ex_cmds2.c 80.67% <100.00%> (-0.49%) ⬇️

... and 128 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/c1470140078@github.com>

Bram Moolenaar

unread,
Mar 16, 2023, 4:15:48 PM3/16/23
to vim/vim, Subscribed


> Currently when one or more edited files are changed outside the editor,
> Vim pops up a dialog asking if the user wants to reload the file.
> Available options are "OK" (ignore), "Load File",
> and "Load File and Option". However, if there are a lot of
> files changed at once (e.g. if you just did a git pull), these dialog
> spams could become quite annoying to deal with as they are modal. You
> can `set autoread` to work around the issue but even then you can only
> set it after you have handled the 30 dialog press and whatnot.
>
> This PR adds an option to reload or ignore all files in one go, to make
> the experience better. Because the way Vim checks mod time is by going
> through files one by file and display the dialog inside the loop, there
> isn't a way to only selectively show the "load all" option only if more
> than one file has been changed right now. Also, this only applies to the
> default check / `:checktime`, and `:diffupdate!`. Other usages of this

> dialog box like `:checktime <buf>` or `:b <buf>` only affect one file
> and will use the old dialog box to avoid confusing the user.
>
> Note: I also changed the mnemonics of "Load File **&a**nd Options" to
> "Load File and O&**p**tion". This may seem less intuitive but I think

> the "Load All" option is actually going to be more useful to most people
> and therefore rebinded "a" to "Load All" instead. I can revert this part
> if desired.

I don't like this much. Making the choice "reload all" or "ignore all"
without knowing what "all" includes is very dangerous. You might think
that all the changes are from some external command, but you can easily
forget another, unrelated file that also got changed (probably longer
ago).

To avoid the long sequence of dialogs, it would be better to list all
the files, set a default choice and then allow for making a different
choice for each entry. We don't have a dialog like that yet, and it
would be quite complicated to implement.

There are some alternatives, such as listing files in the same
directory. With the mentioned "git pull" command one can expect to make
the same choice for all files in a directory, while a file somewhere
else might accidentally be found changed as well. The per-directory
handling makes it more likely the user spots the unexpectedly changed
file.

I think functionality like this deserves a proper solution, also when it
ends up being a lot more work. Not a "quick hack" using a global
variable that needs to be reset.

--
In his lifetime van Gogh painted 486 oil paintings. Oddly enough, 8975
of them are to be found in the United States.

/// 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.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/c1472676837@github.com>

Yee Cheng Chin

unread,
Mar 16, 2023, 5:07:01 PM3/16/23
to vim/vim, Subscribed

Yeah I see your point. My thinking was that when the user has autoread set (which some other text editors already set by default), it's not like they get that choice to begin with, so this "Reload All" option isn't any less unsafe than that (the user is just saying "I don't care, just load me all the files"). We are basically providing a way for the user to set autoread in the middle of the dialogs (which they don't have a way to skip). It does mean it's easier for a user who explicitly has set noautoread to accidentally re-load all files by pressing this.

I do think the current noautoread behavior with the the dialog is quite problematic in certain degenerate cases. This could be really annoying for users who use tabs who may end up having high double-digit open windows (I may have a "friend" who is like that sometimes), and for hidden buffers it creates a downstream effect of every time they try to navigate to a modified hidden buffer they get prompted with whether they want to reload the buffer (which my PR doesn't fix, to be fair).

So basically the issue you have is the unpredictability of the option since you are not given enough information to make an informed decision right? I may need to think about how I want to approach this and look around at other editors so if this isn't going to be merged I may close the PR shortly since I probably won't work on it immediately.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/c1472739724@github.com>

Yee Cheng Chin

unread,
Mar 16, 2023, 5:19:13 PM3/16/23
to vim/vim, Subscribed

One way where the option could definitely be a little iffy is with modified files with unsaved changes. The "Load All" option will load everything including the modified files, which is definitely an issue. We probably don't want to just keep piling top-level buttons ("Reload All", "Reload Unmodified", etc) before it becomes unwieldy as well. This probably needs more thought.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/c1472755991@github.com>

Bram Moolenaar

unread,
Mar 17, 2023, 2:44:11 PM3/17/23
to vim/vim, Subscribed


> One way where the option could definitely be a little iffy is with
> modified files with unsaved changes. The "Load All" option will load
> everything including the modified files, which is definitely an issue.
> We probably don't want to just keep piling top-level buttons ("Reload
> All", "Reload Unmodified", etc) before it becomes unwieldy as well. This
> probably needs more thought.

When the buffer has been changed the user should be asked what to do,
since dropping the changes may mean work is lost.

A better solution could be that 'autoread' is set (locally for the
buffer, not globally) when the file is opened, when some condition is
met. E.g. that the file is under version control. That might not be so
easy or a bit slow though.

--
He was not in the least bit scared to be mashed into a pulp
Or to have his eyes gouged out and his elbows broken;
To have his kneecaps split and his body burned away
And his limbs all hacked and mangled, brave Sir Robin.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD


/// 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.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/c1474261554@github.com>

Yee Cheng Chin

unread,
Mar 17, 2023, 5:59:24 PM3/17/23
to vim/vim, Subscribed

Let me close this for now. Want to think more about it before I iterate on this. I think setting autoread on say any Git repo is already doable with some VImscript / plugin, but it doesn't help the basic problem of someone stuck in a dialog spam, and sometimes the user doesn't want to set autoread to begin with as they may want to be notified of a file change regardless of whether they have modified the file locally.

Another sort-of-related interesting feature in VSCode is that if you try to save a modified file with external mods, it actually allows you to merge it in diff mode instead of just choosing to overwrite the file, which is kind of interesting. Otherwise the default in VSCode is equivalent to set autoread (cannot be turned off), and it just won't re-load modified files with no warning to the user until they try to save (and prompted to diff / overwrite), which I don't like.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/c1474441611@github.com>

Yee Cheng Chin

unread,
Mar 17, 2023, 5:59:26 PM3/17/23
to vim/vim, Subscribed

Closed #12158.


Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/12158/issue_event/8782639027@github.com>

Reply all
Reply to author
Forward
0 new messages