[vim/vim] Reload 'fileformat', 'binary', etc on :checktime! (PR #9579)
Rob Pilling
Currently if a file changes format, encoding or its binary mode outside of vim (e.g. code formatting tools), when reloading it we can end up with changes to the buffer itself, such as carriage returns at the end of each line (etc). This can appear odd to the user, although may be behaviour that's relied upon. We can offer :checktime! to reload these settings too.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/9579
Commit Summary
- 4d35e35 Reload 'fileformat' on :checktime!
File Changes
(14 files)
- M runtime/doc/editing.txt (13)
- M src/buffer.c (2)
- M src/diff.c (2)
- M src/edit.c (2)
- M src/ex_cmds.c (4)
- M src/ex_cmds.h (2)
- M src/ex_cmds2.c (7)
- M src/fileio.c (24)
- M src/main.c (2)
- M src/message.c (2)
- M src/proto/fileio.pro (6)
- M src/spellfile.c (2)
- M src/testdir/test_filechanged.vim (11)
- M src/ui.c (1)
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 #9579 (4d35e35) into master (bed34f0) will decrease coverage by
3.10%.
The diff coverage is88.88%.
@@ Coverage Diff @@ ## master #9579 +/- ## ========================================== - Coverage 83.51% 80.41% -3.11% ========================================== Files 154 152 -2 Lines 174352 172896 -1456 Branches 39222 39238 +16 ========================================== - Hits 145612 139034 -6578 - Misses 16656 21142 +4486 - Partials 12084 12720 +636
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | 81.87% <88.88%> (-0.01%) |
⬇️ |
| huge-gcc-none | ? |
|
| huge-gcc-testgui | ? |
|
| huge-gcc-unittests | 2.03% <0.00%> (-0.01%) |
⬇️ |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/message.c | 79.54% <ø> (-1.49%) |
⬇️ |
| src/spellfile.c | 78.95% <0.00%> (-1.40%) |
⬇️ |
| src/ui.c | 73.65% <ø> (-2.69%) |
⬇️ |
| src/fileio.c | 74.28% <87.50%> (-1.17%) |
⬇️ |
| src/buffer.c | 81.81% <100.00%> (-4.11%) |
⬇️ |
| src/diff.c | 82.11% <100.00%> (-0.86%) |
⬇️ |
| src/edit.c | 82.20% <100.00%> (-3.77%) |
⬇️ |
| src/ex_cmds.c | 84.61% <100.00%> (-1.10%) |
⬇️ |
| src/ex_cmds2.c | 79.51% <100.00%> (-2.16%) |
⬇️ |
| src/main.c | 83.71% <100.00%> (-1.34%) |
⬇️ |
| ... and 140 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 bed34f0...4d35e35. 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 wonder how useful this solution is. First of all, the effect applies to all buffers that need reloading. What if only one specific buffer needs a filetype check?
There is still the dialog when a buffer is reloaded, unless 'autoread' is set.
And in case the check happens not with a :checktime command but a focus event, the user still has to use ":edit" to get the effect.
—
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.
Rob Pilling
Yes, that's true - I suppose if the fileformat and other checks are potentially costly we could make it so a ! is only accepted when a buffer is also specified.
However I suppose I'm sidestepping the main reason I opened the PR. This being that initially the behaviour appeared to me as a bug, where a code-formatting tool changed the fileformat of a few buffers outside of vim (without vim losing focus or suspending - via a terminal buffer, so no dialog/autoread machinery fired), and when I did a :checktime to reload these updated files, the carriage returns were suddenly present on every line, in contrast to :edit.
You're right that stepping through each file manually in vim and doing :e reloaded correctly though.
I now see this is intentional behaviour, I guess I could suspend and resume vim to trigger the reload prompt / autoread rather than doing a :checktime, what do you think?
—
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
Rob Pilling wrote:
> Yes, that's true - I suppose if the fileformat and other checks are
> potentially costly we could make it so a `!` is only accepted when a
> buffer is also specified.
>
> However I suppose I'm sidestepping the main reason I opened the PR.
> This being that initially the behaviour appeared to me as a bug, where
> a code-formatting tool changed the fileformat of a few buffers outside
> of vim (without vim losing focus or suspending - via a terminal
> `:checktime` to reload these updated files, the carriage returns were
>
> You're right that stepping through each file manually in vim and doing
> `:e` reloaded correctly though.
>
> I now see this is intentional behaviour, I guess I could suspend and
> resume vim to trigger the reload prompt / `autoread` rather than doing
The main problem I see is that what is specified applies to all buffers,
there is no choice.
How about doing something with the FileChangedShell autocommand?
Currently it can set v:fcs_choice to "reload". We could add a choice to
reload and detect 'fileformat' and 'filetype' again.
The autocommand can match a path or file name, so that you can do this
for files that are likely to change in a way that detection is needed.
--
ROBIN: The what?
ARTHUR: The Holy Hand Grenade of Antioch. 'Tis one of the sacred relics
Brother Maynard always carries with him.
ALL: Yes. Of course.
ARTHUR: (shouting) Bring up the Holy Hand Grenade!
"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.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Rob Pilling
Yes, I like the sound of that, especially the finer control instead of a big :checktime! hammer.
I can put those changes together - for v:fcs_choice, how does "edit" sound?
—
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, "edit" sounds good. This should then also be an option when "ask" is used.
—
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.
Rob Pilling
@bobrippling pushed 1 commit.
- 9896a14 Reload 'fileformat', etc with v:fcs_choice
You are receiving this because you are subscribed to this thread.
Rob Pilling
@bobrippling pushed 1 commit.
- 3775278 Reload 'fileformat', etc with v:fcs_choice
You are receiving this because you are subscribed to this thread.
Rob Pilling
This is ready for another review - I believe the two test failures for HUGE on Windows are unrelated, and similarly for the others.
—
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
—
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.