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

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

bug#57400: 29.0.50; Support sending patches from VC directly


From: Eli Zaretskii
Subject: bug#57400: 29.0.50; Support sending patches from VC directly
Date: Sat, 15 Oct 2022 09:57:40 +0300

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: juri@linkov.net,  rpluim@gmail.com,  57400@debbugs.gnu.org,  ane@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.

> >>                     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?

> >                                                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 ;-)





reply via email to

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