Re: [PATCHv2] rebase -i: respect core.abbrev for real

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2015, 06:56 UTC
Message-ID: <xmqqsif3uwv7.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1421750552-110075-1-git-send-email-kirill.shutemov@linux.intel.com>

"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> writes:

Show 8 quoted lines

> I have tried to fix this before: see 568950388be2, but it doesn't
> really work.
>
> I don't know how it happend, but that commit makes interactive rebase to
> respect core.abbrev only during --edit-todo, but not the initial todo
> list edit.
>
> For this time I've included a test-case to avoid this frustration again.

I suspect that the work that introduced expand/collapse could have gone one step further so that all of the short object names you are touching in this patch can stay in the full 40-hex name to avoid ambiguity. That is, keep the internal representation always use the full object name, and then always call these three:

    collapse_todo_ids 
    git_sequence_editor
    expand_todo_ids

in this order.

Wouldn't that make the end result a lot cleaner, I wonder?

Show 47 quoted lines

>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  v2: fix &&-chain in the test-case
> ---
>  git-rebase--interactive.sh    | 4 ++--
>  t/t3404-rebase-interactive.sh | 7 +++++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index c6a4629cbc2b..1855e12f1ada 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -962,7 +962,7 @@ else
>  	shortrevisions=$shorthead
>  fi
>  git rev-list $merges_option --pretty=oneline --abbrev-commit \
> -	--abbrev=7 --reverse --left-right --topo-order \
> +	--reverse --left-right --topo-order \
>  	$revisions ${restrict_revision+^$restrict_revision} | \
>  	sed -n "s/^>//p" |
>  while read -r shortsha1 rest
> @@ -1020,7 +1020,7 @@ then
>  			# just the history of its first-parent for others that will
>  			# be rebasing on top of it
>  			git rev-list --parents -1 $rev | cut -d' ' -s -f2 > "$dropped"/$rev
> -			short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev)
> +			short=$(git rev-list -1 --abbrev-commit $rev)
>  			sane_grep -v "^[a-z][a-z]* $short" <"$todo" > "${todo}2" ; mv "${todo}2" "$todo"
>  			rm "$rewritten"/$rev
>  		fi
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 8197ed29a9ec..a31f7e0430e1 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
>  	)
>  '
>  
> +test_expect_success 'respect core.abbrev' '
> +	git config core.abbrev 12 &&
> +	set_cat_todo_editor &&
> +	test_must_fail git rebase -i HEAD~4 >todo-list &&
> +	test 4 = $(grep -c "pick [0-9a-f]\{12,\}" todo-list)
> +'
> +
>  test_done