Kaartic Sivaraam <kaarticsivaraam91...@gmail.com> writes:
> The patch in the previous mail results in a change in output as
> specified below.
>
> $ git branch
> * master
> foo
> bar
>
> Before patch,
>
> $ git branch -m hypothet master
> fatal: A branch named 'master' already exists.
>
> $ git branch -m hypothet real
> error: refname refs/heads/hypothet not found
> fatal: Branch rename failed
>
> After patch,
>
> $ git branch -m hypothet master
> fatal: Branch 'hypothet' does not exist.
>
> $ git branch -m hypothet real
> fatal: Branch 'hypothet' does not exist.
Imagine this scenario instead, which I think is more realistic
example of making a typo. The set of existing branches are like
this:
$ git branch
devel
* test
And then you get these with your patch:
$ git branch -m tets devel
fatal: Branch 'tets' does not exist.
$ git branch -m test devel
fatal: A branch named 'devel' already exists.
My reaction to the above exchange would be a moderately strong
annoyance. If I were told that I am using 'devel' for something
else already, my "corrected" attempt to turn the 'test' branch to a
real development branch after getting the first error would have
been:
$ git branch -m test devel2
and I didn't have to get the second error.
I think your patch points in the right direction---if an operation
can fail due to two reasons, reordering the two checks and still
fail with a report for just the first one you happened to check
first does not give us a real improvement. If it is easy to check
the other one after you noticed one of the condition is violated,
then you could give a more useful diagnosis, namely, "There is
branch 'tets' to rename, and there already is 'devel' branch".
I suspect that with a moderately-sized refactoring around
validate_new_branchname() function, this should be doable. Instead
of passing two "int" parameters force and attr_only, make them into
a single "unsigned flag" and allow the caller to pass another option
to tell the function "do not die, do not issue a message, but tell
the caller what is wrong with a return value". And by using that
updated API, rename_branch() could tell four cases apart and fail
the three bad cases in a more sensible way.
In any case, the illustrations of interaction before and after the
change is a very good thing to have when discussing a patch, and you
are encouraged to put them in your proposed log message.
