GNU bug report logs - #57400
29.0.50; Support sending patches from VC directly

Previous Next

Full log

Message #334 received at 57400 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rpluim <at> gmail.com, 57400 <at> debbugs.gnu.org, ane <at> iki.fi, juri <at> linkov.net
Subject: Re: bug#57400: 29.0.50; Support sending patches from VC directly
Date: Sat, 15 Oct 2022 11:44:58 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: juri <at> linkov.net,  rpluim <at> gmail.com,  57400 <at> debbugs.gnu.org,  ane <at> iki.fi
>> Date: Fri, 14 Oct 2022 21:47:43 +0000
>> 
>> >   Translate Git revision descriptor of COMMIT, a string, to a symbolic form.
>> 
>> Is this perhaps not too long?  Would "Translate revision string COMMIT
>> to a symbolic form." be sufficient, especially as this is actually just
>> an internal function that wasn't marked as such (the Git history
>> indicates that this function, with this documentation string has been
>> around since the initial revision of the file in 2007)?
>
> I disregarded the fact that this is an internal function, because the
> issues are general.  But yes, we could make this shorter, once we
> agree that the full text should be something like I suggested.  For
> example:
>
>   Translate revision string of COMMIT to a symbolic form.
>
> Note that I said "string of COMMIT", because COMMIT is not a string,
> it is an entity which the string describes.

Right, makes sense.

>> >>                     If the optional argument FORCE is non-nil,
>> >> this might include revision specifications like \"master~8\" (the
>> >> 8th parent of the commit that \"master\" is currently pointing
>> >> to).
>> >
>> > This begs the question: what kind of COMMIT strings are acceptable if
>> > FORCE is nil or omitted?  If we only accept SHA-1 hashes then, this
>> > should perhaps be mentioned in the first sentence.  But from reading
>> > the (unhelpful) man page of "git name-rev" (which leads down the
>> > rabbit hole to "git rev-parse"), it is my understanding that this will
>> > accept _any_ revision descriptor in any form.  
>> 
>> Yes, right.  And in absence of any restriction "COMMIT" should be
>> understood to be a SHA-1 reference or a symbolic reference, right?
>
> Hmm... now I'm confused.  I'd think COMMIT could be _any_ reference to
> a revision, not just SHA-1?  Because "git name-rev" accepts them all,
> no?

Yes?  Maybe I am also confused, but my understanding is that a "commit
reference" (e.g. the contents of COMMIT) is either a symbolic reference
of a SHA-1 reference (non-symbolic).

>> >                                                So now I wonder why
>> > accepting something like "master~8" needs a special knob: it's just
>> > one of the forms supported by "git name-rev", isn't it?  So maybe you
>> > don't even need the additional argument and don't have to document it?
>> 
>> That is an open debate, the function is currently only used in vc-git.el
>> and is never invoked with the optional argument.  I've only added it
>> because it might be that it could be useful at some point in the future.
>> 
>> > But if there _are_ valid reasons not to accept the likes of
>> > "master~8", they should be in the doc string.  For example:
>> >
>> >   By default, COMMIT strings of the form "master~8" are rejected,
>> >   because <describe the reason here>, but if FORCE is non-nil, they
>> >   are allowed.
>> 
>> I guess it is difficult to come up with a "valid reason", the motivation
>> is that I wanted to have some way to ensure that
>> `vc-git-working-revision' only returns a symbolic form iff a branch or
>> tag is pointing to the working revision.  If you think it is preferable,
>> I could also invert the argument and make it into something like
>> "no-relative" or even pull the check into `vc-git-working-revision'.
>
> I'm asking why not accept everything that "git name-rev" accepts, and
> remove the need for the additional FORCE argument.  (But this is not a
> documentation issue anymore ;-)

The function does accept everything that "git name-rev" accepts, after
all it just passes commit to the subcommand.  What FORCE does is
restrict what "git name-rev" response is accepted to be returned by
`vc-git-symbolic-commit'.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.