patchgit-rebase: Print name of rev when using shorthand

The output from a successful invocation of the shorthand command "git rebase -" is something like "Fast-forwarded HEAD to @{-1}", which includes a relative reference to a revision. Other commands that use the shorthand "-", such as "git checkout -", typically display the symbolic name of the revision.

Change rebase to output the symbolic name of the revision when using the shorthand. For the example above, the new output is "Fast-forwarded HEAD to master", assuming "@{-1}" is a reference to "master".

- Use "git name-rev" to retreive the name of the rev.
- Update the tests in light of this new behavior.

Requested-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Brian Gesiak <modocache@gmail.com>
---
Previous discussion on this issue:
http://article.gmane.org/gmane.comp.version-control.git/244340

 git-rebase.sh     | 2 +-
 t/t3400-rebase.sh | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

Brian Gesiak <modocache@gmail.com> writes:

What the patch wants to implement sounds sensible, but I do not think name-rev is a right tool for this. Imagine the case where there are more than one branches whose tip points at the commit you came from. name-rev will not be able to pick correctly which one to report.

Also think what happens if you were previously on a detached HEAD?

I think you would want to use something like:

        upstream_name=$(git rev-parse --symbolic-full-name @{-1})
        if test -n "$upstream"
        then
                upstream_name=${upstream_name#refs/heads/}
        else
                upstream_name="@{-1}"
        fi

if the change is to be made at that point in the code.

I also wonder if "git rebase @{-1}" deserve a similar translation like you are giving "git rebase -".

OK.

Thank you for the feedback!

I see. Yes, you're exactly right; the following demonstrates the problem:

$ git checkout -b xylophone master $ git checkout -b aardvark master $ git name-rev --name-only @{-1} # I'd want "xylophone", but this outputs "aardvark"

So it appears name-rev is not up to the task here.

I agree, I will re-roll the patch to use this approach.

Personally, I've been using the "-" shorthand with "git checkout" for a year or so, but only learned about "@{-1}" a few months ago. I think those who use "@{-1}" are familiar enough with the concept that they don't need to have the reference translated to a symbolic full name. Users familiar with "-" might not be aware of "@{-1}", however, so I'd prefer not to output it as we are currently.

Furthermore, were we to translate "@{-1}", does that mean we should also translate "@{-2}" or prior? I don't think that's the case, but then only translating "@{-1}" would seem inconsistent. From that point of view I'd prefer to simply translate "-", not "@{-1}".

- Brian Gesiak

Brian Gesiak <modocache@gmail.com> writes:

I do not understand that reasoning.

The concept of "n-th prior checkout" (aka @{-n}) and "immediately previous checkout" (aka "-") are equivalent, even though the former may be more generic.

You seem to be saying that those who understand the former are with superiour mental capacity in general than those who only know the latter, and they can always remember where they came from. It sounds similar to an absurd claim (pulled out of thin-air only for illustration purposes) that French-speaking people are of superiour mind and do not need as much help with math as English speakers.

Surely, why not. If a user is so forgetful to need help remembering where s/he was immediately before, wouldn't it be more helpful to give "here is where you were" reminder for older ones to allow them to double check they specified the right thing and spot possible mistakes?

I can buy "that would be a lot more work, and I do not want to do it (or I do not think I can solve it in a more general way)", though.

Junio C Hamano <gitster@pobox.com> writes:

After re-reading the proposed log message of your v2, I notice one thing:

    The output from a successful invocation of the shorthand command
    "git rebase -" is something like "Fast-forwarded HEAD to @{-1}",
    which includes a relative reference to a revision. Other
    commands that use the shorthand "-", such as "git checkout -",
    typically display the symbolic name of the revision.
  
While the above is not incorrect per-se, have you considered _why_
it is a good thing to show the symbolic name in the first place?

Giving the symbolic name 'master' is good because it is possible that the user thought the previous branch was 'frotz', forgetting that another branch was checked out tentatively in between, and the user ended up rebasing on top of a wrong branch. Telling what that previous branch is is a way to help user spot such a potential mistake. So I am all for making "rebase -" report what concrete branch the branch was replayed on top of, and consider it an incomplete improvement if "rebase @{-1}" (or "rebase @{-2}") did not get the same help---especially when I know that the underlying mechanism you would use to translate @{-1} back to the concrete branch name is the same for both cases anyway.

By the way, here is a happy tangent. I was pleasantly surprised to see what this procedure produced:

    $ git checkout -b ef/send-email-absolute-path maint
    $ git am -s3c a-patch-by-erik-on-different-topic
    $ git checkout bg/rebase-off-of-previous-branch
    $ git am -s3c your-v2-patch
    $ git checkout jch
    $ git merge --no-edit -
    $ git merge --no-edit @{-2}
    $ git log --first-parent -2 | grep "Merge branch"

Both short-hands are turned into concrete branch names, as they should ;-)

I think I failed to express my point here; I don't think people that use "@{-1}" have superior mental capacity, but rather simply that they are aware of the "@{-n}" method of specifying a previous reference. So in response to the command "git rebase @{-4}", displaying the result "Fast-forwarded HEAD to @{-4}" does not contain any unknown syntax that may confuse them. They may not remember what "@{-4}" refers to, but they are aware of the syntax at least.

On the other hand, people who use the "-" shorthand may or may not be aware of the "@{-n}" syntax. In that respect, I think it would be confusing to display "Fast-forwarded HEAD to @{-1}" in response to the command "git rebase -"; the user may not know what "@{-1}" means!

Thus my original point was that I felt displaying a symbolic name in response to "git rebase -" was more important than doing so in response to "git rebase @{-1}". The issue isn't about forgetting what "@{-n}" refers to, it's whether the user even knows what "@{-n}" is supposed to mean.

But in light of your other comments:

I had not originally thought of this, perhaps because I was preoccupied with preventing users from seeing syntax they might not be aware of. But I definitely agree that displaying symbolic names for all "@{-n}" is a good way to prevent user error.

Perish the thought! :)

I will try to re-roll this patch to include symbolic names for "@{-n}".

As usual, thanks for the feedback!

- Brian Gesiak