[vim/vim] :w increments b:changedtick, but ":w foo" does not (#2764)

[Daniel Hahler]

I've noticed that :write will increment b:changedtick always (and worked around / handled this in Neomake, where this is handled/used in BufWritePost).

But then I've noticed that fugitive's manual triggering of BufWritePost does not increment it, so it becomes inconsistent there already.

Additionally, specifying a filename (:w /tmp/foo) also does not increment it.

Therefore I think that incrementing it with :w only should be considered to be a bug.
In Neomake I could then check for the patch that fixes it to skip the workaround, instead of adding more workarounds.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub

[Bram Moolenaar]

Daniel Hahler wrote:

> I've noticed that `:write` will increment `b:changedtick` always (and worked around / handled this in Neomake, where this is handled/used in `BufWritePost`).

>
> But then I've noticed that fugitive's manual triggering of `BufWritePost` does not increment it, so it becomes inconsistent there already.
>

> Additionally, specifying a filename (`:w /tmp/foo`) also does not increment it.

>
> Therefore I think that incrementing it with `:w` only should be considered to be a bug.
> In Neomake I could then check for the patch that fixes it to skip the workaround, instead of adding more workarounds.

That :write changes b:changedtick is because the state of the buffer
changed, it is no longer seen as modified. If you write under another
name this doesn't happen. Thus it's not the write command itself that
matters, it's that the buffer becomes not-modified.

--
hundred-and-one symptoms of being an internet addict:
87. Everyone you know asks why your phone line is always busy ...and
you tell them to send an e-mail.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Daniel Hahler]

I see.
However it happens unconditionally, even if it was not modified before.

It makes it hard to detect if a buffer's content was changed (for linting purposes).

[Bram Moolenaar]

Daniel Hahler wrote:

> I see.
> However it happens unconditionally, even if it was not modified before.
>
> It makes it hard to detect if a buffer's content was changed (for
> linting purposes).

For that we have the :update command.

In corner cases :write even has effect when the buffer didn't change,
e.g.:
:!mv % %.bak
:write

--
hundred-and-one symptoms of being an internet addict:

91. It's Saturday afternoon in the middle of May and you
are on computer.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—

[Daniel Hahler]

For that we have the :update command.

This will increment b:changedtick however in the same way.
The point / use case is detecting if the buffer's contents was changed, and therefore b:changedtick should be the same before and after :w / :up.

In corner cases :write even has effect when the buffer didn't change,

Good point, however not relevant to the buffer's contents.
Neomake uses temporary files (or stdin) to handle unsaved buffers, and when it is triggered / used with TextChanged and BufWritePost it does not make sense to lint again on BufWritePost then.

[Bram Moolenaar]

Daniel Hahler wrote:

> > For that we have the :update command.
>
> This will increment `b:changedtick` however in the same way.

No it doesn't. Edit some file without changing anything:
:edit file
:echo b:changedtick
3
:update
:echo b:changedtick
3
:write
:echo b:changedtick
4

> The point / use case is detecting if the buffer's contents was
> changed, and therefore `b:changedtick` should be the same before and

> after `:w` / `:up`.

It doesn't really work for that, even &modified is sometimes set when
the buffer didn't actually change. E.g. using "rx" on a character that
already is an "x". Vim doesn't really check the text before and after.

> > In corner cases :write even has effect when the buffer didn't change,
>
> Good point, however not relevant to the buffer's contents.
> Neomake uses temporary files (or stdin) to handle unsaved buffers, and
> when it is triggered / used with TextChanged and BufWritePost it does
> not make sense to lint again on `BufWritePost` then.

--
From "know your smileys":
~#:-( I just washed my hair, and I can't do nuthin' with it.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—

[Daniel Hahler]

:update will increment b:changedtick however in the same way.

No it doesn't.

:update without any changes does not trigger BufWritePost at all (which is good), and does not update the tick, yes.
But if there are changes (and saving the buffer is triggered) it increases the tick additionally when being called.
My point here is that :normal ofoo sets tick to X, but :update then to X+1, althought the contents was not changed.

It doesn't really work for that

Is there something that can be used then?
b:changedtick would be good if this issue was fixed.
The following looks like some trivial fix, but would likely need more adjustments internally (and changes the behavior of course):

diff --git i/src/misc1.c w/src/misc1.c
index 135293dd2..a307c20d5 100644
--- i/src/misc1.c
+++ w/src/misc1.c
@@ -3208,8 +3208,8 @@ unchanged(
 #ifdef FEAT_TITLE
        need_maketitle = TRUE;      /* set window title later */
 #endif
+       ++CHANGEDTICK(buf);
     }
-    ++CHANGEDTICK(buf);
 #ifdef FEAT_NETBEANS_INTG
     netbeans_unmodified(buf);
 #endif

There is b_last_changedtick as a special tick used for the TextChanged event - maybe this could be exposed? (added in 7.4.493, https://github.com/vim/vim/blob/e7499ddc33508d3d341e96f84a0e7b95b2d6927c/src/fileio.c#L4996-L5000) - but I'd rather not have a new variable, which is very similar to an existing one.

As for Neomake I could hook into TextChanged always when BufWritePost is used, and use that tick then instead of b:changedtick, but it would be nice if this was not necessary.

OTOH, vim-fugitive appears to be the only plugin I am using that does 'doautocmd BufWritePost' manually, so special-casing this (and others to come) might be good enough.

[Daniel Hahler]

If the current is to be kept, then why doesn't it happen with :w /tmp/foo (using a name)?
Is it because unchanged() is not called then?

[Bram Moolenaar]

Daniel Hahler wrote:

> > > `:update` will increment `b:changedtick` however in the same way.

>
> > No it doesn't.
>
> `:update` without any changes does not trigger `BufWritePost` at all (which is good), and does not update the tick, yes.
> But if there are changes (and saving the buffer is triggered) it increases the tick additionally when being called.
> My point here is that `:normal ofoo` sets tick to X, but `:update` then to X+1, althought the contents was not changed.

As mentioned, b:changedtick is not just for making changes but also for
when the undo/redo or 'modified' state changes.

> > It doesn't really work for that
>
> Is there something that can be used then?
> `b:changedtick` would be good if this issue was fixed.
> The following looks like some trivial fix, but would likely need more adjustments internally (and changes the behavior of course):

Perhaps you can check the value of 'modified' in BufWritePre?

--
From "know your smileys":

:-)-O Smiling doctor with stethoscope

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—

[Christian Brabandt]

I am closing this as working as designed.

[Christian Brabandt]

Closed #2764.

[Daniel Hahler]

As mentioned, b:changedtick is not just for making changes but also for when the undo/redo or 'modified' state changes.

As for 'modified' that's not the case however: :set modified does not change b:changedtick.

As for undo/redo that's expected, since it changes the buffer (usually).

I still fail to see why triggering the BufWritePost autocommand is considered to be changing the state of the buffer?
It still looks like a bug/inconsistency to me.

Perhaps you can check the value of 'modified' in BufWritePre?

I want to know if the buffer was (not) changed, not if it is modified.

The problem is that Vim unconditionally increments b:changedtick when executing BufWritePost itself, which I am working around - but then this workaround causes a false positive for when BufWritePost is executed via doautocmd (e.g. in a BufWriteCmd with vim-fugitive), where b:changedtick is not incremented and Neomake then assumes the buffer was not changed, if b:changedtick is only off by one (which is the case with :diffget for example).

When using :normal ofoo it will increment b:changedtick by 2, so the buffer is considered to be changed also after adjusting b:changedtick by 1 for BufWritePost.

I cannot see why/how looking at &modified in BufWritePre might help here.

Keep in mind that I do not want to know if the buffer is/was modified, but if it changed since the last time a check was run.

What is wrong with the patch I've posted above? (created a WIP PR based on it: #4505)

Vim itself appears to use a similar workaround in v7.4.493 - which maybe might not have been necessary with the other patch.

[chdiza]

Keep in mind that I do not want to know if the buffer is/was modified, but if it changed since the last time a check was run.

Then couldn't you use the new listener_*() stuff instead? I thought this is exactly what the listener stuff was for.

There is b_last_changedtick as a special tick used for the TextChanged event - maybe this could be exposed?

Maybe that var could be made accessible with test_getvalue().

[Daniel Hahler]

Then couldn't you use the new listener_*() stuff instead? I thought this is exactly what the listener stuff was for.

Thanks for the hint, however Neomake also supports Neovim and older Vims, where this would not be available (which is also the case for a bugfix patch however, of course).
But the main reason to not use it is that I am not interested in re-acting to changes directly, and would only use it to track a custom b:my_changedtick then effectively. And therefore I think it makes more sense to fix b:changedtick instead.

There is b_last_changedtick as a special tick used for the TextChanged event - maybe this could be exposed?

Maybe that var could be made accessible with test_getvalue().

I do not think this is a good idea, and might even be used internally as a workaround only, similar to what was done in 086329d (v7.4.493):

+#ifdef FEAT_AUTOCMD
+       /* buf->b_changedtick is always incremented in unchanged() but that
+        * should not trigger a TextChanged event. */
+       if (last_changedtick + 1 == buf->b_changedtick
+                                              && last_changedtick_buf == buf)
+           last_changedtick = buf->b_changedtick;
+#endif

Anyway, tests might be passing by now with #4505, so discussion could continue there, based on tests / actual behavior (changes).

[chdiza]

Bram keeps telling you that b:changedtick is (despite the name) intentionally not supposed to exactly track what you're calling "changes", and yet you keep insisting on making it do that.

But the main reason to not use it is that I am not interested in re-acting to changes directly,

This makes no sense at all. You're reacting either way.

and would only use it to track a custom b:my_changedtick then effectively.

So?

[Daniel Hahler]

Bram keeps telling you that b:changedtick is (despite the name) intentionally not supposed to exactly track what you're calling "changes", and yet you keep insisting on making it do that.

I got that, and can live with it being increment for e.g. undo etc - i.e. I am not trying to exactly track changes through it.

But I cannot see why it should be incremented unnecessarily, and inconsistenly, which makes it hard to work around / use it then.

Also I got the impression that my use case was not really understood,

The/my main issue here is that it gets incremented additionally when not using BufWriteCmd, but not without it.

Apart from that and from what I can see my proposed patch would also make other internal workarounds not be required in the first place, and if the patch is not wanted, then at least the changetick should also be incremented when using BufWriteCmd/doauto BufWritePost then for consistency - adding another special case / condition in the code, which might not be needed in the first place.

and would only use it to track a custom b:my_changedtick then effectively.

So?

I would rather use TextChanged/TextChangedI then I guess, but all of them are adding overhead.

I am trying to improve Vim here instead of working around its issues...!

[Bram Moolenaar]

[...]

> Also I got the impression that my use case was not really understood,
>
> The/my main issue here is that it gets incremented additionally when

> not using `BufWriteCmd`, but not without it.

I haven't seen a remark about how this happens. An autocmd by itself
doesn't do it, is it what the autocommand executes?

> Apart from that and from what I can see my proposed patch would also
> make other internal workarounds not be required in the first place,
> and if the patch is not wanted, then at least the changetick should

> also be incremented when using `BufWriteCmd`/`doauto BufWritePost`

> then for consistency - adding another special case / condition in the
> code, which might not be needed in the first place.
>
> > > and would only use it to track a custom b:my_changedtick then effectively.
> >
> > So?
>

> I would rather use `TextChanged`/`TextChangedI` then I guess, but all

> of them are adding overhead.
>
> I am trying to improve Vim here instead of working around its issues...!

There are a few calls to unchanged() that should increment the
changedtick even when the buffer was not changed before. For example
when editing the same buffer again and the file text has changed.

When writing a file that doesn't reset the changed flag, one could
argue that changedtick should not be incremented, since no text actually
changed. When the changed flag is reset it's probably OK to increment
changedtick, even though the text itself hasn't changed. The buffer
state did change.

The idea with changedtick is that it's better to err on the safe side,
incrementing a bit too often would not be so bad.

Anyway, you dropped your pull request, thus I suppose we don't really
need that accuracy.

--
hundred-and-one symptoms of being an internet addict:

116. You are living with your boyfriend who networks your respective
computers so you can sit in separate rooms and email each other

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—

[Daniel Hahler]

Thanks for your explanation.

I haven't seen a remark about how this happens. An autocmd by itself doesn't
do it, is it what the autocommand executes?

The confusing thing for me was that using BufWriteCmd (manual BufWritePost
via doautocmd, used by vim-fugitive) would not increment b:changedtick,
but using no BufWriteCmd does so.

(and then additionally that :diffget only increments it by one, while
:normal foo by 2; which basically means that then it would run into the false
positive due to the offset being one, and Neomake not triggering the maker,
because the tick was not changed after the adjustment for BufWritePost)

I can see and understand that err'ing on the safe side is good in general.

And yeah, I got annoyed by having spent too much time on this yet again (the
issue itself is older already).

[Bram Moolenaar]

> Thanks for your explanation.
>
> > I haven't seen a remark about how this happens. An autocmd by itself doesn't
> > do it, is it what the autocommand executes?
>
> The confusing thing for me was that using `BufWriteCmd` (manual `BufWritePost`

> via `doautocmd`, used by vim-fugitive) would not increment `b:changedtick`,

> but using no `BufWriteCmd` does so.
>
> (and then additionally that `:diffget` only increments it by one, while
> `:normal foo` by 2; which basically means that then it would run into the false
> positive due to the offset being one, and Neomake not triggering the maker,
> because the tick was not changed after the adjustment for BufWritePost)
>
> I can see and understand that err'ing on the safe side is good in general.
>
> And yeah, I got annoyed by having spent too much time on this yet again (the
> issue itself is older already).

Well, I could not increment changedtick when writing a file when
'changed' was off already. Then the buffer does not really change in
any way. But perhaps that fix then breaks your logic?

--
An actual excerpt from a classified section of a city newspaper:
"Illiterate? Write today for free help!"

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—

[Daniel Hahler]

No, that fix would be good I think - I've removed the BufWritePost workaround already (since it does not work with BufWriteCmd).
Not incrementing the tick in general would make it behave as desired then, i.e. the BufWritePost handler considers the file to not be changed, if it was handled before already.

[Bram Moolenaar]

> No, that fix would be good I think - I've removed the BufWritePost
> workaround already (since it does not work with BufWriteCmd).
> Not incrementing the tick in general would make it behave as desired
> then, i.e. the `BufWritePost` handler considers the file to not be
> changed, if it was handled before already.

OK, I'll make a change to unchanged() similar to what you had. But
there are a few places where unchanged() is called where it makes sense
to increment b:changedtick always. I'll add an argument to force that.
Hopefully I get it right.

--
hundred-and-one symptoms of being an internet addict:

124. You begin conversations with, "Who is your internet service provider?"

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—

[Bram Moolenaar]

> No, that fix would be good I think - I've removed the BufWritePost
> workaround already (since it does not work with BufWriteCmd).
> Not incrementing the tick in general would make it behave as desired
> then, i.e. the `BufWritePost` handler considers the file to not be
> changed, if it was handled before already.

This is included in patch 8.1.1498. Let me know if you still see a
related problem.

While writing the test, which is in patch 8.1.1501, I made a difference
between "normal o" and "normal otext", so you can see the first tick is
for inserting a line, and the second tick is for actually inserting
something. I think that's OK.

--
hundred-and-one symptoms of being an internet addict:

137. You decide to stay in college for an additional year or two,
just so you can have the free Internet access.