[PATCH] branch and tag subcommands don't accept colons in labels/names

[Sune Foldager]

On 24-11-2011 14:45, Fabian Kreutz wrote:
> Hi!
>
>> There is already some branch name verification going on in
>> dirstate.setbranch():
>> ...so maybe it should be put there instead.
>
> Good point. The command itself is frontend only.
> But do we want to impose the same restriction e.g. on the convert
> command (that also creates branches)? Isn't it easier to allow
> all non-interactive commands to create whatever branch (as long as
> it does not jeopardize integrity) and then suffer the error that I
> detail below?
> I'm not familiar with the API. Would a user of the hg-API use the
> branch command or the repo.setbranch method?

The branch command calls into dirstate.setbranch to do the actual work, 
so all calls to change the branch end up there.

>> Also, I think we generally
>> put an empty line between the summary and the rest of the commit
>> message, and keep the summary lower-case.
> *sigh*
> I'm sorry.
> I hearby allow the person that copy-pastes the change into the actual
> repository to do the appropriate formatting changes and promise not
> to do the same errors again in 2012.

I guess I came off much more serious than I intended :p.

>> In general, I think we should be careful with adding new limitations to
>> these things, since existing repos could already use them. We use '/' in
>> branch names (where I work), for instance. Of course, if it's
>> documented, it's another matter I guess.
>
> Not only is it documented, it will simply fail later.
> You can create a branch with an invalid name, but you cannot update to it.
> The error message depends on the exact format, but in our case it
> was something like:
>
> hg up "ISSUE-123: some long explaination"
> parsing error in char 16
> (which is the second whitespace)
>
> There is a different message for names that contain a : but no space.

I believe some of this is due to revsets. Maybe

hg up "branch('ISSUE-123: some long explaination')"

will work? I think every obscure branch name can be reached that way. 
Also, some commands (pull, push) have --branch/-b arguments which only 
match branch names and not revsets, so they should work as well.

>> Why not plain and simple
>>       if ':' not in label:
>> ?
> I don't see a difference in readbility or performance.
> Personally I associate "in" with lists (of which a string is a member - I know)
> and .find with strings. So "find" clarifies while reading that we are
> operating on what must be a string.

I don't really care either way, but maybe see what's done in other 
places in the Mercurial code.

/Sune