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

[Michael Albinus <michael.albinus@gmx.de>]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04/10/2016 11:00 AM, Michael Albinus wrote:
>
>> If there is a simple and obvious fix for this problem, let apply it to
>> the emacs-25 branch. Even if we must change it later in master.
>
> Why not revert 7f9b037245ddb662ad98685e429a2498ae6b7c62? I believe
> I've mentioned that suggestion before.

Why that? That patch fixes problems. How would you solve them otherwise?

And what problem has been introduced with that patch? This one we are
speaking about, bug#20637? Maybe you have mentioned this already, and
I've overseen this.

> The only difficulty here, as far as I'm concerned, is to update the
> corresponding tests so they don't break, but are not disabled, and
> still look somewhat reasonable. That's where the merge conflict
> concerns come into play. But "no disabled tests" is not a hard
> requirement for release anyway.

Could you show a patch which reverts
7f9b037245ddb662ad98685e429a2498ae6b7c62, and also updates the tests?
Maybe it is simpler to speak about this concrete proposal.

>> But I doubt that's possible. This bug report was written on 23 May 2015,
>> it was marked as release blocker on the same day, and still no fix.
>
> Not because it's too difficult to resolve.

But nobody has taken any action for more than 10 months :-(

>> For all those problems in vc I have the impression, that we must
>> stabilize vc first. Starting with the very basic functionality - that's
>> why I've added tests for vc-backend and vc-responsible-backend.
>
> I agree with the sentiment, but not with certain choices of the areas
> to "stabilize" it in. You've basically been discovering aspects of the
> current commands and functions that are poorly specified. But those
> aspects (with some exceptions, I suppose) are not regressions from
> Emacs 24. They have been there for a while.

Oh, there are regressions. ESR has undertaken his rewrite of vc*.el end
of 2014. Emacs 25.1 is the first release which brings them to the
public.

See also vc.el, and compare the functions listed in the Commentary
section between Emacs 24.5 and 25.1. Read also "Changes from the
pre-25.1 API" at the same place in the 25.1 version of vc.el.

Unfortunately, etc/NEWS of Emacs 25.1 is very silent about those
changes.

>> I don't care whether we discuss it here, in bug#19548 or in the
>> emacs-devel ML.
>
> I don't mind too much any way, but 19548 is for the manual and such. A
> separate bug report (or several) or discussions on emacs-devel seem
> preferable. Unless I'm mistaken about these problems being orthogonal,
> of course.

I might be wrong, but Glenn didn't write this bug only because of some
missing docs. I believe he was unhappy about the whole process, how
those changes did arrive. But I'm not Glenn, and I shouldn't interpret
too much ...

>> ... I even doubt that directories are out of scope of vc-backend.
>> Directories can be registered for some VCS, for example for CVS, or for
>> ClearCase which I use at work (I know, we have no official support for
>> ClearCase in vc).
>
> In general, VC supports the lowest common denominator across the
> backends. Or at least, a feature should be supported in a few
> important ones. CVS is on its way out, and ClearCase is a relatively
> niche tool.

You have read my comment in vc-tests.el? At least CVS, RCS and SVN seem
to behave this way. So it is reasonable to handle the case, that
directories are registered.

> Anyway, the point is VC is for people to be able to write code
> depending on the public API and see it work across many VCSes. And Git
> and Mercurial, at least, don't track directories.

So what? We could say in the doc that registering directories is not
supported for all backends. People who write a backend know what they
do. I hope.

>> Therefore we shall precise the docstring of
>> vc-backend, that it returns the backend also for directories for VCSes
>> which support rgistration of directories.
>
> Why? Just tell anyone who wants to know a directory's backend to use
> vc-responsible-backend instead.

I have no problem to recommend vc-responsible-backend. But vc-backend
does already TRT: when a directory is registered, it returns the backend
name. When a directory is not registered, it returns nil. This is *not*
a statement why a directory is not registered, and whether registering
directories is possible at all for a backend.

>> I know what the docstring says :-) My question is, whether we shall
>> offer the same argument list for both vc-backend and vc-responsible-backend.
>
> We could, but honestly, that question doesn't sound particularly
> important right now. Yes, it's a wart, and if there are no users that
> pass a list to it, we can remove this possibility.
>
> Note that vc-backend and vc-responsible-backend have different
> performance characteristics (and, I imagine, different behavior in
> case of nested repositories), so simply replacing all uses of the
> former with the latter is not a good idea.

I haven't proposed this! I simply want to understand (and document) what
both functions are doing. Which of them is taken by a backend is the
decision of the maintainer of the backend.

I'm pretty much in favor to also document the differences in performance. 

>> Don't know. But I believe "interactively" is not the criterion whether a
>> general vc function shall exist. I also don't call vc-registered
>> interactively.
>
> Interactive use would be a strong justification. vc-register can be
> used interactively, and it's called from vc-next-action.

I haven't spoken about vc-register. I have spoken about
vc-registered. This one exist as general function, and it isn't interactive.

> How will we use vc-unregister?

Every backend can use it instead of its backend specific function, like
vc-git-unregister. It runs common code then like removing properties,
and calls the backend specific function then.

>>>> It should call
>>>>   common code, like vc-file-clearprops. For the time being, I have
>>>>   emulated this.
>>>
>>> Are you doing that just to test the `unregister' implementations?
>>
>> Yes.
>
> That seems very low priority. I wonder how often vc-transfer-file gets
> used in practice these days.

I understand that's low priority for you, and I accept this. For me,
while extending vc-tests.el, it isn't low priority. I need solid ground
under my feed, and therefore I'm starting with the very basic
functions. Understanding, testing, maybe fixing bugs, maybe improving
documentation. Function by function.

>> And I'm still in favor of adding vc-unregister in vc.el.
>
> I don't really mind.

So I will do, unless somebody stops me :-)

Best regards, Michael.