All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>, git@vger.kernel.org
Subject: Re: [PATCHv3] rebase -i: respect core.abbrev for real
Date: Thu, 22 Jan 2015 12:10:04 -0800	[thread overview]
Message-ID: <xmqq3872tw4j.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1421927415-114643-1-git-send-email-kirill.shutemov@linux.intel.com> (Kirill A. Shutemov's message of "Thu, 22 Jan 2015 13:50:15 +0200")

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

> 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.
>
> The patch change code to use full 40-hex revision ids for todo actions
> everywhere and collapse them only to show to user.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  v3:
>     - use full 40-hex revision ids for todo actions everywhere and collapse them
>       only to show to user;

> @@ -1054,6 +1052,7 @@ has_action "$todo" ||
>  	return 2
>  
>  cp "$todo" "$todo".backup
> +collapse_todo_ids
>  git_sequence_editor "$todo" ||
>  	die_abort "Could not execute editor"
>  

OK, the matching expand_todo_ids is just beyond the horizon of this
patch context.

I was hoping that with this change we internally operate with the
full object names throughout the program, only to show shortened
ones in the editor, but I still see a handful of "rev-parse --short"
outside collapse_todo_ids.  Upon closer inspection, it turns out
that they are only about formatting "# Rebase a..b onto c", which is
never rewritten in transform/collapse/expand_todo_ids, so I think
all is well.

Thanks.

> 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

  reply	other threads:[~2015-01-22 20:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-19 14:20 [PATCH] rebase -i: respect core.abbrev for real Kirill A. Shutemov
2015-01-19 22:43 ` Eric Sunshine
2015-01-20 10:42   ` [PATCHv2] " Kirill A. Shutemov
2015-01-22  6:56     ` Junio C Hamano
2015-01-22 11:50   ` [PATCHv3] " Kirill A. Shutemov
2015-01-22 20:10     ` Junio C Hamano [this message]
2015-01-22 20:19     ` RFC "grep '...\{m,n\}"? Junio C Hamano
2015-01-23  0:39       ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq3872tw4j.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.