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

[Dmitry Gutov <dgutov@yandex.ru>]

Sorry, forgot to cc.

On 01.07.2012 13:58, Michael Albinus wrote:
> Dmitry Gutov <dgutov@yandex.ru> 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