bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#11757: Acknowledgement (24.1.50; vc-git calls `process-file' too man


From: Dmitry Gutov
Subject: bug#11757: Acknowledgement (24.1.50; vc-git calls `process-file' too many times)
Date: Sun, 01 Jul 2012 18:58:01 +0400
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1

Sorry, forgot to cc.

On 01.07.2012 13:58, Michael Albinus wrote:
Dmitry Gutov <address@hidden> writes:

I'm not sure what we should do. Call (vc-mode-line) anyway? That would work.

Nope. This is expansive, because it recomputes `vc-working-revision'. We
would loose all improvements from using the cache.

I don't think so.

If we hadn't reset all properties in vc-before-save (file existed),
nothing changes.
If we did reset them, then yes, vc-working-revision will recompute
vc-working-revision property, but only once after the reset.
Which is what we want to do anyway, since the file's state has
changed, and the working revision could have changed as well, so we
need to know them to update mode-line.

Likely, the best option is to call (vc-registered file) after clearing
the file cache. This recomputes the 'vc-backend property as well, what
we want.

Have you tried this? (vc-backend file) gets called just a bit later, in `vc-after-save`, and it calls `vc-registered' in turn, but since the file is no longer registered with Git, it returns nil, and the mode-line is not updated.

Also, I don't think we should call (vc-backend file) before the file is written to disk.

Calling (vc-mode-line file) at this point would be for the side-effect
of that function, which is bad in my experience. It would harden
maintenance, 'cause nobody will know why we want to refresh the modeline
at this point.

I meant, always calling (vc-mode-line file) in `vc-after-save', moving it outside of the (and backend ...) check. This does have a performance downside with non-VC file buffers, of a couple additional function calls, 'vc-backend cache lookup, and unconditional mode-line update.

So from this standpoint, saving and restoring 'vc-backend value around clearing props in `vc-before-save' might indeed be the best solution.

By the way, this last patch I sent doesn't help if the user just
removed the file from repository while leaving it on disk (git rm
--cached ... && git commit ..., for example), but whatever.

If we use the cache, there will always be a constellation that the cache
is stale due to external operations. As Stefan said, this is mostly
uncritical.

I'm bringing attention to these cases because they could be the actual benefit of your patch (caching 'git-registered) over my initial patch (not calling `vc-git-registered' from `vc-git-state' at all). The original code supported these edge cases, at the cost of additional process call.

-- Dmitry





reply via email to

[Prev in Thread] Current Thread [Next in Thread]