Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Mar 25, 2018, 07:36 UTC
Message-ID: <42ca98f1-e916-a159-27fe-02137f73a525@gmail.com>
In-Reply-To: <20180325054824.GA56795@flurp.local>

On Sunday 25 March 2018 11:18 AM, Eric Sunshine wrote:

Show 14 quoted lines

> On Sun, Mar 25, 2018 at 09:11:34AM +0530, Kaartic Sivaraam wrote:
>> On Sunday 25 March 2018 07:04 AM, Eric Sunshine wrote:
>>> Can we have a couple new tests: one checking "git branch --list" for
>>> the typical case (when rebasing off a named branch) and one checking
>>> when rebasing from a detached HEAD?
>>
>> Sure, but I guess it would take some time for me to add the tests. I'll
>> send a v2 with the suggested changes.
> 
> A couple more comments:
> 
> * Please run the commit message through a spell checker; it contains
>   several typographical errors.
>

Thanks for motivating me to search for a spell checker. I have now discovered the spell check feature (:set spell) in Vim!

Show 13 quoted lines

> * I wonder if it makes sense to give slightly different output in the
>   detached HEAD case. Normal output is:
> 
>       (no branch, rebasing <branch>)
> 
>   and, with your change, detached HEAD output is:
> 
>       (no branch, rebasing d3adb33f)
> 
>   which is okay, but perhaps it could be better; for instance:
> 
>       (no branch, rebasing detached HEAD d3adb33f)
>

I just recently discovered that the variable used to print information related to detached HEAD (state.detached_from) might also contain remote branch names (origin/master, etc.) other than commit hashes. So, it might make sense to distinguish detached HEAD.

> Anyhow, I wrote the tests for you. When you re-roll, you can make the
> following patch 2/2 and your fix 1/2.

Thanks a lot!

Show 69 quoted lines

> (If you go with the above idea
> of using a slightly different wording for the detached HEAD case, then
> you'll need to adjust the 'grep' slightly in the second test.)
> 
> --- >8 ---
> From: Eric Sunshine <sunshine@sunshineco.com>
> Date: Sun, 25 Mar 2018 01:29:58 -0400
> Subject: [PATCH] t3200: verify "branch --list" sanity when rebasing from
>  detached HEAD
> 
> "git branch --list" shows an in-progress rebase as:
> 
>   * (no branch, rebasing <branch>)
>     master
>     ...
> 
> However, if the rebase is started from a detached HEAD, then there is no
> <branch>, and it would attempt to print a NULL pointer. The previous
> commit fixed this problem, so add a test to verify that the output is
> sane in this situation.
> 
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t3200-branch.sh | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 6c0b7ea4ad..d1f80c80ab 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -6,6 +6,7 @@
>  test_description='git branch assorted tests'
>  
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY"/lib-rebase.sh
>  
>  test_expect_success 'prepare a trivial repository' '
>  	echo Hello >A &&
> @@ -1246,6 +1247,29 @@ test_expect_success '--merged is incompatible with --no-merged' '
>  	test_must_fail git branch --merged HEAD --no-merged HEAD
>  '
>  
> +test_expect_success '--list during rebase' '
> +	test_when_finished "reset_rebase" &&
> +	git checkout master &&
> +	FAKE_LINES="1 edit 2" &&
> +	export FAKE_LINES &&
> +	set_fake_editor &&
> +	git rebase -i HEAD~2 &&
> +	git branch --list >actual &&
> +	grep "rebasing master" actual
> +'
> +
> +test_expect_success '--list during rebase from detached HEAD' '
> +	test_when_finished "reset_rebase && git checkout master" &&
> +	git checkout HEAD^0 &&
> +	oid=$(git rev-parse --short HEAD) &&
> +	FAKE_LINES="1 edit 2" &&
> +	export FAKE_LINES &&
> +	set_fake_editor &&
> +	git rebase -i HEAD~2 &&
> +	git branch --list >actual &&
> +	grep "rebasing $oid" actual
> +'
> +
>  test_expect_success 'tracking with unexpected .fetch refspec' '
>  	rm -rf a b c d &&
>  	git init a &&
>

-- 
Kaartic