Hi,
Junio C Hamano wrote:
> Things like @{-1} would not make any sense when the command is run
> outside a repository, and the documentation is quite clear that it
> is the primary reason why we added "--branch" option to the command,
> i.e.
>
> With the `--branch` option, it expands the ``previous branch syntax''
> `@{-n}`. For example, `@{-1}` is a way to refer the last branch you
> were on. This option should be used by porcelains to accept this
> syntax anywhere a branch name is expected, so they can act as if you
> typed the branch name.
>
> So I am tempted to take this patch to make sure that we won't gain
> more people who abuse the command outside a repository.
That seems very sensible on its face. My only worry is that a script
that can be run both inside and outside a repository and does
branch=$(git check-ref-format --branch "$user_supplied_branch_arg")
currently works with user_supplied_branch_arg='master' and would stop
working. If we have reason to believe that no such scripts exist,
then this would be a good way to go, but I don't believe we can count
on that.
And in that spirit, I think the patch you replied with aims to go in
the right direction, by providing the core functionality when in a
repository while avoiding breaking such a script outside of one
(though I do not understand it fully yet).
> Having said that, there may still be a use case where a Porcelain
> script wants a way to see if a $name it has is appropriate as a
> branch name before it has a repository
This seems like a different goal than "git check-ref-format --branch"
was originally designed to fulfill (even though it fits well with the
check-ref-format name and coincides with --branch behavior when in a
repository). I think it's fine for us not to fulfill it.
> (e.g. a wrapper to "git
> clone" that wants to verify the name it is going to give to the "-b"
> option), and a check desired in such a context is different from
> (and is stricter than) feeding refs/heads/$name to the same command
> without the "--branch" option.
Can you say more about this example? E.g. why is this hypothetical
wrapper unable to rely on "git clone -b"'s own error handling?
> So I think the right endgame in the longer term is:
>
> - Find (or add if it doesn't exist) a way to recommend to Porcelain
> scripts to use to expand an end-user generated string, and to map
> it to a branch name (it may be "rev-parse --symbolic-full-name
> $name"; I dunno).
--symbolic-full-name seems like a good fit. Do you remember why
check-ref-format was introduced instead? Was it just a matter of
implementation simplicity, since --symbolic-full-name can handle a
broader class of revision specifications like --remotes? The commit
message to v1.6.3-rc0~29^2~4 (give Porcelain a way to grok branch
shorthand, 2009-03-21) is appropriately apologetic but doesn't give
more clues.
> - Keep check-ref-format as (or revert it to be) a tool to "check".
> This would involve split strbuf_check_branch_ref() into two:
Without an example of where this tool would be used, if we consider
"check-ref-format --branch" to be a mistake then I'd rather deprecate
it with a goal of removing it completely.
Ok, time to look in more detail.
Thanks for your thoughtfulness,
Jonathan
