Brian Gesiak <modoca...@gmail.com> writes:

> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 6d94b1f..6176754 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -88,6 +88,17 @@ test_expect_success 'rebase from ambiguous branch name' '
>       git rebase master
>  '
>  
> +test_expect_success 'rebase using shorthand' '
> +     git checkout master &&
> +     git checkout -b shorthand HEAD^ &&
> +     git rebase - 1>shorthand.stdout &&
> +     git checkout master &&
> +     git branch -D shorthand &&
> +     git checkout -b shorthand HEAD^ &&
> +     git rebase @{-1} 1>without_shorthand.stdout &&
> +     test_i18ncmp without_shorthand.stdout shorthand.stdout
> +'

A handful of issues here:

 * "1>target" looks unconventional and wastes readers' time, forcing
   them to wonder if there is anything special going on, only to
   realize there isn't anything noteworthy.  Saying ">target" like
   everybody else does avoids attracting unnecessary attention.

 * "rebase using shorthand" is somewhat a myopic title; it assumes
   that the only short-hand relevant to rebase will be that a "-"
   stands for "@{-1}" to specify the branch we rebase the current
   branch off of.

 * The usual filename for the output from the command being tested
   is 'actual', and the usual filename for the expected output is
   'expect'.  In this case, you are verifying that the output from
   "rebase -" is the same as the output from "rebase @{-1}", so it
   is more conventional to call the former 'actual' and the latter
   'expect'.

 * Is the eye-candy output to the standard output what is the most
   interesting during the execution of a rebase?  Wouldn't we be
   more interested to make sure that we did transplant the history
   on the same commit between two cases?

   "rebase -" with your change still says something like this:

        First, rewinding head to replay your work on top of it...
        Fast-forwarded HEAD to @{-1}.

   instead of "Fast-forwarded HEAD to -".  Somebody may later want
   to "fix" this, making these two eye-candy output to be different
   from each other, and what your test expects will no longer hold
   (not that I think it is better to say "-" instead of @{-1}
   there).


I'll tentatively queue it with a minor tweak (see below).

Thanks.

-- >8 --
From: Brian Gesiak <modoca...@gmail.com>
Date: Wed, 19 Mar 2014 20:02:15 +0900
Subject: [PATCH] rebase: allow "-" short-hand for the previous branch

Teach rebase the same shorthand as checkout and merge to name the
branch to rebase the current branch on; that is, that "-" means "the
branch we were previously on".

Requested-by: Tim Chase <g...@tim.thechases.com>
Signed-off-by: Brian Gesiak <modoca...@gmail.com>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
 git-rebase.sh     |  4 ++++
 t/t3400-rebase.sh | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/git-rebase.sh b/git-rebase.sh
index 8a3efa2..658c003 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -449,6 +449,10 @@ then
                test "$fork_point" = auto && fork_point=t
                ;;
        *)      upstream_name="$1"
+               if test "$upstream_name" = "-"
+               then
+                       upstream_name="@{-1}"
+               fi
                shift
                ;;
        esac
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6d94b1f..80e0a95 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,23 @@ test_expect_success 'rebase from ambiguous branch name' '
        git rebase master
 '
 
+test_expect_success 'rebase off of the previous branch using "-"' '
+       git checkout master &&
+       git checkout HEAD^ &&
+       git rebase @{-1} >expect.messages &&
+       git merge-base master HEAD >expect.forkpoint &&
+
+       git checkout master &&
+       git checkout HEAD^ &&
+       git rebase - >actual.messages &&
+       git merge-base master HEAD >actual.forkpoint &&
+
+       test_cmp expect.forkpoint actual.forkpoint &&
+       # the next one is dubious---we may want to say "-",
+       # instead of @{-1}, in the message
+       test_i18ncmp expect.messages actual.messages
+'
+
 test_expect_success 'rebase a single mode change' '
        git checkout master &&
        git branch -D topic &&
-- 
1.9.1-423-g4596e3a

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

Reply via email to