bug#20637: incompatible, undocumented change to vc-working-revision

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> I haven't thought too much about performance. But you are right, we
>> shouldn't add serious performance penalties to the code. And improving
>> performance for the 25.1 release is much too late.
>
> It's hard for me to judge how serious those are, really (I only have a
> fast laptop with GNU/Linux these days), but being wary of extra
> process calls seems prudent. Ideally, we'd reduce their number, not
> increase it.

Yes. I hope we could use more file properties caches. To be investigated.

>> So we might revert the patch for vc-state and vc-working-revision indeed
>> for the emacs-25 branch, going back to using vc-backend.
>
> Thanks, I agree.

I've committed this to emacs-25. Plus commenting the now failing checks
in vc-tests.el.

>> In the master branch we might apply my proposed patch using
>> vc-registered or something similar, and start to improve performance.
>
> Improve how? Would you like to comment on the last paragraph of my
> previous email in this subthread?

You've proposed an interface change in vc-state, returning just nil
instead of unregistered. Yes, this might be an option. Worth to be
checked.

Another possibility is to use cached properties more aggressively, as
said above.

> I don't really see a point in returning `unregistered' from
> `vc-state'. When would the caller treat it differently from nil? And
> returning nil seems like an easier choice, implementation-wise, and
> well as a more conservative one from the backward compatibility
> perspective.
>
> The `dir-status-files' backend command would continue including the
> `unregistered' entries (we could make it skip the up-to-date ones,
> though, in the interest of improving performance).

I cannot comment about this today. And as said already several times, if
we would document vc-* functions in the manual, it would allow us to
have a more global view on proposed changes. I trust you that you have
all involved interfaces in your mind. I haven't, and I would like to see
how an interface change compares to the other interfaces.

>> In
>> parallel, we shall start to write a VCS section for the elisp manual,
>> describing vc-* functionality in more detail. We could start with
>> vc-backend and vc-responsible-backend and their intended use. I'm
>> missing such documentation for years.
>
> I'd rather put the missing information into the docstrings, really. It
> seems unlikely that we're missing more than a few sentences in these
> two functions' descriptions, and we could also rephrase the existing
> ones.
>
> But if you'd be more comfortable with having that information in the
> manual as well, don't let me stop you.

Maybe docstrings are already sufficient. But you have spoken about
design decisions in the past (for example whether unregistered files
could be an argument), which I believe is not documented.

And at least for me the "global view" about vc-* functions is missing,
and how they are related.

>> I'll come back later today with the patch for emacs-25, if you agree.
>
> In any case, I definitely agree with reverting vc-state and
> vc-working-revision to use vc-backend in Emacs 25.1.

Yep. Pls test my patch, and confirm whether it is sufficient. Same for
Glenn, if possible. I would like to close this bug then, removing a
release blocker for Emacs 25.1.

Best regards, Michael.