Re: [PATCH] tag: do not show ambiguous tag names as "tags/foo"

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

 



On Mon, Jan 25, 2016 at 12:21:21PM -0800, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > I'm not sure "remove-standard-prefix" doesn't open its own questions.
> > Like "what are the standard prefixes?".
> 
> It is easy to define, no?  This is invented for the internal use of
> the listing modes of "tag" and "branch", so the users are welcome to
> use it if they see fit, but how the prefixes are stripped is defined
> by the convenience of these commands--the behaviour might even change
> when these commands are enhanced.

What does this do:

  git for-each-ref --format='%(refname:remove-standard-prefix)'

?

Is there no standard prefix, because we are not in branch/tag? Does it
remove well-known prefixes like "refs/heads/", "refs/tags/", etc? Does
it remove the first two components (e.g., what happens to
"refs/foo/bar")? If so, what happens to "refs/stash"?

We can say "it is all internally defined and subject to change". But
that is not very helpful to a user, and this "it's magic, don't rely on
it" wart will be part of the user-facing interface. We have to write
_something_ in the "default format" documentation for git-tag.

> Yeah, I really do not like any of the "do not error out but assume
> that the user would not care about the ambiguities" solutions for
> something we primarily intend to use for internal purposes.
> 
> I agree that "strip=<n>", "remove-prefix=<glob>", and the friends
> are good for end-user scripting, but they can come later, outside of
> the scope of this regression fix, and that is the proper time to
> debate and decide if it is really ok to assume that the user does
> not care about ambiguity, or it is prudent to error out.  A separate
> "remove-standard-prefix" that is meant for internal use would allow
> us to push the fix out without having to decide now.

Yeah, I am definitely on board with trying to do the most minimal thing
for the regression fix and worry about more advanced concerns later (if
at all). It seems to me that "error out" is a pretty minimal behavior,
though, and one that doesn't lock us into any behaviors (i.e., it is
generally OK to take something that did not work at all before, and give
it new useful behavior; it is not OK to change something that did
something useful before).

So what about this:

  1. Implement ":strip=<n>" and use it from git-tag.

  2. Have it error out on a ref with fewer than <n> components. This
     should be impossible to trigger via "git-tag" with the default
     format.

  3. Explicitly document that the behavior for values of <n> that are
     not positive integers is undefined and subject to change (or
     alternatively, we can simply error out).

That _is_ user-visible, but it seems like "strip" is a fairly flexible
mechanism by itself (enough that I do not mind living with it forever),
and we haven't made any promises about the ambiguous parts.


If we are going to do something _really_ internal and undocumented to
fix the regression, I think I would rather not introduce a new formatter
at all, but simply teach "%(refname:short)" to do the magic internal
shortening in the context of git-tag. That does not let people emulate
"tag" with "for-each-ref", but that is not part of the regression or its
fix.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]