git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Add "git rebase --show-patch"
@ 2018-01-26  9:55 Nguyễn Thái Ngọc Duy
  2018-01-26  9:55 ` [PATCH 1/2] am: add --show-patch Nguyễn Thái Ngọc Duy
                   ` (5 more replies)
  0 siblings, 6 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-26  9:55 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

When a conflict happens during a rebase, you often need to look at the
original patch to see what the changes are. This requires opening your
favourite pager with some random path inside $GIT_DIR.

This series makes that experience a bit better, by providing a command
to read the patch. This is along the line of --edit-todo and --quit
where you can just tell git what to do and not bother with details.

My main focus is "git rebase", but because rebase uses "git am" behind
the scene, "git am" gains --show-patch option too.

There was something more I wanted to do, like coloring to the patch.
But that probably will come later. I'll try to merge these two
21-months-old patches first.

Nguyễn Thái Ngọc Duy (2):
  am: add --show-patch
  rebase: add --show-patch

 Documentation/git-am.txt               |  5 +++-
 Documentation/git-rebase.txt           |  5 +++-
 builtin/am.c                           | 23 ++++++++++++++---
 contrib/completion/git-completion.bash |  6 ++---
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  4 +++
 git-rebase--merge.sh                   |  4 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 34 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  6 +++++
 t/t4150-am.sh                          |  5 ++++
 11 files changed, 93 insertions(+), 9 deletions(-)

-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 1/2] am: add --show-patch
  2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
@ 2018-01-26  9:55 ` Nguyễn Thái Ngọc Duy
  2018-01-26  9:55 ` [PATCH 2/2] rebase: " Nguyễn Thái Ngọc Duy
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-26  9:55 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
around in there, which is not a good thing. With this, the user does
not have to keep the path around somewhere (because after a couple of
commands, the path may be out of scrollback buffer) when they need to
look at the patch. Bonus point for automatic pager.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  5 ++++-
 builtin/am.c                           | 23 ++++++++++++++++++++---
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          |  5 +++++
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..724095e921 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-patch)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,9 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--show-patch::
+	Show the patch being applied.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..aaaf32886a 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
 			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
 
 			if (advice_amworkdir)
-				printf_ln(_("The copy of the patch that failed is found in: %s"),
-						am_path(state, "patch"));
+				printf_ln(_("Use 'git am --show-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
@@ -2121,6 +2120,17 @@ static void am_abort(struct am_state *state)
 	am_destroy(state);
 }
 
+static void show_patch(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	if (strbuf_read_file(&sb, am_path(state, msgnum(state)), 0) >= 0) {
+		setup_pager();
+		write_in_full(1, sb.buf, sb.len);
+	}
+	strbuf_release(&sb);
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2159,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2249,6 +2260,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "show-patch", &resume,
+			N_("show the patch being applied."),
+			RESUME_SHOW_PATCH),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2359,6 +2373,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_SHOW_PATCH:
+		show_patch(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..1e9105f6d5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --show-patch"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..eff6455f42 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am pauses on conflict' '
 	test -d .git/rebase-apply
 '
 
+test_expect_success 'am --show-patch' '
+	git am --show-patch >actual.patch &&
+	test_cmp .git/rebase-apply/0001 actual.patch
+'
+
 test_expect_success 'am --skip works' '
 	echo goodbye >expected &&
 	git am --skip &&
-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 2/2] rebase: add --show-patch
  2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
  2018-01-26  9:55 ` [PATCH 1/2] am: add --show-patch Nguyễn Thái Ngọc Duy
@ 2018-01-26  9:55 ` Nguyễn Thái Ngọc Duy
  2018-01-26 11:12   ` Phillip Wood
  2018-01-26 19:11   ` Eric Sunshine
  2018-01-26 18:47 ` [PATCH 0/2] Add "git rebase --show-patch" Tim Landscheidt
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-26  9:55 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

It is useful to see the full patch while resolving conflicts in a
rebase. The only way to do it now is

    less .git/rebase-*/patch

which could turn out to be a lot longer to type [1] if you are in a
linked worktree, or not at top-dir. On top of that, an ordinary user
should not need to peek into .git directory. The new option is
provided to examine the patch.

[1] A conflict caused by git-rebase--am.sh does show the path to this
    patch file so you could copy/paste. But then after some time and
    lots of commands to resolve the conflict, that path is likely
    scrolled out of your terminal.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           |  5 +++-
 contrib/completion/git-completion.bash |  4 +--
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  4 +++
 git-rebase--merge.sh                   |  4 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 34 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  6 +++++
 8 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..4fd571d393 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream> [<branch>]]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort | --quit | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo | --show-patch
 
 DESCRIPTION
 -----------
@@ -250,6 +250,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --edit-todo::
 	Edit the todo list during an interactive rebase.
 
+--show-patch::
+	Show the current patch in an interactive rebase.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1e9105f6d5..b70da4990f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,11 +1992,11 @@ _git_rebase ()
 {
 	__git_find_repo_path
 	if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
-		__gitcomp "--continue --skip --abort --quit --edit-todo"
+		__gitcomp "--continue --skip --abort --quit --edit-todo --show-patch"
 		return
 	elif [ -d "$__git_repo_path"/rebase-apply ] || \
 	     [ -d "$__git_repo_path"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort --quit"
+		__gitcomp "--continue --skip --abort --quit --show-patch"
 		return
 	fi
 	__git_complete_strategy && return
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..564a4a5830 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-patch)
+	exec git am --show-patch
+	;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..01cc002efd 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -840,6 +840,10 @@ To continue rebase after editing, run:
 
 	exit
 	;;
+show-patch)
+	cmt="$(cat "$state_dir/stopped-sha")"
+	exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+	;;
 esac
 
 comment_for_reflog start
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..5c513a9736 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,10 @@ skip)
 	finish_rb_merge
 	return
 	;;
+show-patch)
+	cmt="$(cat "$state_dir/current")"
+	exec git format-patch --subject-prefix= --stdout "${cmt}^!"
+	;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..ce17090520 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
+show-patch!        show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--quit|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo|--show-patch)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -412,6 +413,10 @@ quit)
 edit-todo)
 	run_specific_rebase
 	;;
+show-patch)
+	run_specific_rebase
+	die "BUG: run_specific_rebase is not supposed to return here"
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 8ac58d5ea5..599f0d5606 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -277,4 +277,38 @@ EOF
 	test_cmp From_.msg out
 '
 
+test_expect_success 'rebase--am.sh and --show-patch' '
+	test_create_repo conflict-apply &&
+	(
+		cd conflict-apply &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --onto init HEAD^ &&
+		git rebase --show-patch >actual.patch &&
+		test_cmp .git/rebase-apply/0001 actual.patch
+	)
+'
+
+test_expect_success 'rebase--merge.sh and --show-patch' '
+	test_create_repo conflict-merge &&
+	(
+		cd conflict-merge &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --merge --onto init HEAD^ &&
+		git rebase --show-patch >actual.patch &&
+		cmt=$(cat .git/rebase-merge/current) &&
+		git format-patch --stdout --subject-prefix= ${cmt}^! >expected.patch &&
+		test_cmp expected.patch actual.patch
+	)
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 481a350090..40f70a0333 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -225,6 +225,12 @@ test_expect_success 'stop on conflicting pick' '
 	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
+test_expect_success 'show conflicted patch' '
+	git rebase --show-patch >actual.patch &&
+	git format-patch --subject-prefix= --stdout new-branch1^! >expected.patch &&
+	test_cmp expected.patch actual.patch
+'
+
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/2] rebase: add --show-patch
  2018-01-26  9:55 ` [PATCH 2/2] rebase: " Nguyễn Thái Ngọc Duy
@ 2018-01-26 11:12   ` Phillip Wood
  2018-01-26 11:22     ` Duy Nguyen
  2018-01-26 19:11   ` Eric Sunshine
  1 sibling, 1 reply; 40+ messages in thread
From: Phillip Wood @ 2018-01-26 11:12 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy, git

On 26/01/18 09:55, Nguyễn Thái Ngọc Duy wrote:
> 
> It is useful to see the full patch while resolving conflicts in a
> rebase. The only way to do it now is
> 
>     less .git/rebase-*/patch
> 
> which could turn out to be a lot longer to type [1] if you are in a
> linked worktree, or not at top-dir. On top of that, an ordinary user
> should not need to peek into .git directory. The new option is
> provided to examine the patch.
> 
> [1] A conflict caused by git-rebase--am.sh does show the path to this
>     patch file so you could copy/paste. But then after some time and
>     lots of commands to resolve the conflict, that path is likely
>     scrolled out of your terminal.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-rebase.txt           |  5 +++-
>  contrib/completion/git-completion.bash |  4 +--
>  git-rebase--am.sh                      |  3 +++
>  git-rebase--interactive.sh             |  4 +++
>  git-rebase--merge.sh                   |  4 +++
>  git-rebase.sh                          |  7 +++++-
>  t/t3400-rebase.sh                      | 34 ++++++++++++++++++++++++++
>  t/t3404-rebase-interactive.sh          |  6 +++++
>  8 files changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 8a861c1e0d..4fd571d393 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  	[<upstream> [<branch>]]
>  'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
>  	--root [<branch>]
> -'git rebase' --continue | --skip | --abort | --quit | --edit-todo
> +'git rebase' --continue | --skip | --abort | --quit | --edit-todo | --show-patch
>  
>  DESCRIPTION
>  -----------
> @@ -250,6 +250,9 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>  --edit-todo::
>  	Edit the todo list during an interactive rebase.
>  
> +--show-patch::
> +	Show the current patch in an interactive rebase.
> +
>  -m::
>  --merge::
>  	Use merging strategies to rebase.  When the recursive (default) merge
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1e9105f6d5..b70da4990f 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1992,11 +1992,11 @@ _git_rebase ()
>  {
>  	__git_find_repo_path
>  	if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
> -		__gitcomp "--continue --skip --abort --quit --edit-todo"
> +		__gitcomp "--continue --skip --abort --quit --edit-todo --show-patch"
>  		return
>  	elif [ -d "$__git_repo_path"/rebase-apply ] || \
>  	     [ -d "$__git_repo_path"/rebase-merge ]; then
> -		__gitcomp "--continue --skip --abort --quit"
> +		__gitcomp "--continue --skip --abort --quit --show-patch"
>  		return
>  	fi
>  	__git_complete_strategy && return
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 14c50782e0..564a4a5830 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -27,6 +27,9 @@ skip)
>  	move_to_original_branch
>  	return
>  	;;
> +show-patch)
> +	exec git am --show-patch
> +	;;
>  esac
>  
>  if test -z "$rebase_root"
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index d47bd29593..01cc002efd 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -840,6 +840,10 @@ To continue rebase after editing, run:
>  
>  	exit
>  	;;
> +show-patch)
> +	cmt="$(cat "$state_dir/stopped-sha")"
> +	exec git format-patch --subject-prefix= --stdout "${cmt}^!"
> +	;;
>  esac
>  
>  comment_for_reflog start
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 06a4723d4d..5c513a9736 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -137,6 +137,10 @@ skip)
>  	finish_rb_merge
>  	return
>  	;;
> +show-patch)
> +	cmt="$(cat "$state_dir/current")"
> +	exec git format-patch --subject-prefix= --stdout "${cmt}^!"
> +	;;
>  esac

Here and in the git-rebase--interactive you have access to the SHA of
the failed pick so you could run git log --patch and git colored output
and it would use the pager in the same way as 'git am --show-patch' does

Best Wishes

Phillip

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/2] rebase: add --show-patch
  2018-01-26 11:12   ` Phillip Wood
@ 2018-01-26 11:22     ` Duy Nguyen
  2018-01-30 11:15       ` Phillip Wood
  0 siblings, 1 reply; 40+ messages in thread
From: Duy Nguyen @ 2018-01-26 11:22 UTC (permalink / raw)
  To: phillip.wood; +Cc: Git Mailing List

On Fri, Jan 26, 2018 at 6:12 PM, Phillip Wood <phillip.wood@talktalk.net> wrote:
>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
>> index 06a4723d4d..5c513a9736 100644
>> --- a/git-rebase--merge.sh
>> +++ b/git-rebase--merge.sh
>> @@ -137,6 +137,10 @@ skip)
>>       finish_rb_merge
>>       return
>>       ;;
>> +show-patch)
>> +     cmt="$(cat "$state_dir/current")"
>> +     exec git format-patch --subject-prefix= --stdout "${cmt}^!"
>> +     ;;
>>  esac
>
> Here and in the git-rebase--interactive you have access to the SHA of
> the failed pick so you could run git log --patch and git colored output

Yes. My first revision I actually did "git diff" here. The only
problem is inconsistency because we can't color "git am --show-patch"
the same way, the patch source is in text format, not in the repo. But
if people are ok with that I sure would switch to "git show".

> and it would use the pager in the same way as 'git am --show-patch' does

format-patch does set up pager. If it does not I would be very
annoyed. I added this for convenience after all.
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
  2018-01-26  9:55 ` [PATCH 1/2] am: add --show-patch Nguyễn Thái Ngọc Duy
  2018-01-26  9:55 ` [PATCH 2/2] rebase: " Nguyễn Thái Ngọc Duy
@ 2018-01-26 18:47 ` Tim Landscheidt
  2018-01-27  1:45   ` Duy Nguyen
  2018-01-27 11:26 ` Ævar Arnfjörð Bjarmason
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 40+ messages in thread
From: Tim Landscheidt @ 2018-01-26 18:47 UTC (permalink / raw)
  To: git

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:

> When a conflict happens during a rebase, you often need to look at the
> original patch to see what the changes are. This requires opening your
> favourite pager with some random path inside $GIT_DIR.

> This series makes that experience a bit better, by providing a command
> to read the patch. This is along the line of --edit-todo and --quit
> where you can just tell git what to do and not bother with details.

> My main focus is "git rebase", but because rebase uses "git am" behind
> the scene, "git am" gains --show-patch option too.

> There was something more I wanted to do, like coloring to the patch.
> But that probably will come later. I'll try to merge these two
> 21-months-old patches first.

> […]

I dislike the approach to use a separate command/option.
The nice thing about rebase-apply/original-commit is that
you can use it in /any/ git command, i. e. you can do "git
log $whatever..rebase-apply/original-commit".

What I would do instead is (besides documenting it :-)) to
provide an alias that is more in line with ORIG_HEAD,
FETCH_HEAD, etc.; i. e. something along the lines of (pseudo
code, will probably not work):

| --- a/builtin/am.c
| +++ b/builtin/am.c
| @@ -1110,6 +1110,7 @@ static void am_next(struct am_state *state)
|  
|         oidclr(&state->orig_commit);
|         unlink(am_path(state, "original-commit"));
| +       delete_ref(NULL, "ORIG_COMMIT", NULL, 0);
|  
|         if (!get_oid("HEAD", &head))
|                 write_state_text(state, "abort-safety", oid_to_hex(&head));
| @@ -1441,6 +1442,7 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
|  
|         oidcpy(&state->orig_commit, &commit_oid);
|         write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
| +       update_ref_oid("am", "ORIG_COMMIT", &commit_oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
|  
|         return 0;
|  }

This (when working) would allow to use ORIG_COMMIT in place
of the mouthful rebase-apply/original-commit.

Tim


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/2] rebase: add --show-patch
  2018-01-26  9:55 ` [PATCH 2/2] rebase: " Nguyễn Thái Ngọc Duy
  2018-01-26 11:12   ` Phillip Wood
@ 2018-01-26 19:11   ` Eric Sunshine
  2018-01-27  1:42     ` Duy Nguyen
  1 sibling, 1 reply; 40+ messages in thread
From: Eric Sunshine @ 2018-01-26 19:11 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: Git List

On Fri, Jan 26, 2018 at 4:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> It is useful to see the full patch while resolving conflicts in a
> rebase. The only way to do it now is
>
>     less .git/rebase-*/patch
>
> which could turn out to be a lot longer to type [1] if you are in a
> linked worktree, or not at top-dir. On top of that, an ordinary user
> should not need to peek into .git directory. The new option is
> provided to examine the patch.
> [...]
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> @@ -840,6 +840,10 @@ To continue rebase after editing, run:
> +show-patch)
> +       cmt="$(cat "$state_dir/stopped-sha")"
> +       exec git format-patch --subject-prefix= --stdout "${cmt}^!"
> +       ;;

What is the behavior if a rebase (or conflicted rebase) is not in
progress? Stated differently, do we only make it this far if
$state_dir/stopped_sha exists?

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/2] rebase: add --show-patch
  2018-01-26 19:11   ` Eric Sunshine
@ 2018-01-27  1:42     ` Duy Nguyen
  0 siblings, 0 replies; 40+ messages in thread
From: Duy Nguyen @ 2018-01-27  1:42 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List

On Sat, Jan 27, 2018 at 2:11 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Fri, Jan 26, 2018 at 4:55 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> It is useful to see the full patch while resolving conflicts in a
>> rebase. The only way to do it now is
>>
>>     less .git/rebase-*/patch
>>
>> which could turn out to be a lot longer to type [1] if you are in a
>> linked worktree, or not at top-dir. On top of that, an ordinary user
>> should not need to peek into .git directory. The new option is
>> provided to examine the patch.
>> [...]
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> @@ -840,6 +840,10 @@ To continue rebase after editing, run:
>> +show-patch)
>> +       cmt="$(cat "$state_dir/stopped-sha")"
>> +       exec git format-patch --subject-prefix= --stdout "${cmt}^!"
>> +       ;;
>
> What is the behavior if a rebase (or conflicted rebase) is not in
> progress? Stated differently, do we only make it this far if
> $state_dir/stopped_sha exists?

It belongs to the same --continue/--skip/--abort group and won't reach
here unless a rebase is in progress (i.e. $state_dir exists).
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-26 18:47 ` [PATCH 0/2] Add "git rebase --show-patch" Tim Landscheidt
@ 2018-01-27  1:45   ` Duy Nguyen
  0 siblings, 0 replies; 40+ messages in thread
From: Duy Nguyen @ 2018-01-27  1:45 UTC (permalink / raw)
  To: Git Mailing List

On Sat, Jan 27, 2018 at 1:47 AM, Tim Landscheidt <tim@tim-landscheidt.de> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
>> When a conflict happens during a rebase, you often need to look at the
>> original patch to see what the changes are. This requires opening your
>> favourite pager with some random path inside $GIT_DIR.
>
>> This series makes that experience a bit better, by providing a command
>> to read the patch. This is along the line of --edit-todo and --quit
>> where you can just tell git what to do and not bother with details.
>
>> My main focus is "git rebase", but because rebase uses "git am" behind
>> the scene, "git am" gains --show-patch option too.
>
>> There was something more I wanted to do, like coloring to the patch.
>> But that probably will come later. I'll try to merge these two
>> 21-months-old patches first.
>
>> […]
>
> I dislike the approach to use a separate command/option.
> The nice thing about rebase-apply/original-commit is that
> you can use it in /any/ git command, i. e. you can do "git
> log $whatever..rebase-apply/original-commit".
>
> What I would do instead is (besides documenting it :-)) to
> provide an alias that is more in line with ORIG_HEAD,
> FETCH_HEAD, etc.; i. e. something along the lines of (pseudo
> code, will probably not work):
>
> | --- a/builtin/am.c
> | +++ b/builtin/am.c
> | @@ -1110,6 +1110,7 @@ static void am_next(struct am_state *state)
> |
> |         oidclr(&state->orig_commit);
> |         unlink(am_path(state, "original-commit"));
> | +       delete_ref(NULL, "ORIG_COMMIT", NULL, 0);
> |
> |         if (!get_oid("HEAD", &head))
> |                 write_state_text(state, "abort-safety", oid_to_hex(&head));
> | @@ -1441,6 +1442,7 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
> |
> |         oidcpy(&state->orig_commit, &commit_oid);
> |         write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
> | +       update_ref_oid("am", "ORIG_COMMIT", &commit_oid, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
> |
> |         return 0;
> |  }
>
> This (when working) would allow to use ORIG_COMMIT in place
> of the mouthful rebase-apply/original-commit.

Interesting. Taken out of context, I don't think it really works for
"git am" because the source patches are in text form and "git am" only
fails when it can't apply the patch (and won't be able to create a
commit). But for rebase I can see how this is a good thing. Thanks.
Let me sleep on it for a bit. It works for cherry-pick too.
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2018-01-26 18:47 ` [PATCH 0/2] Add "git rebase --show-patch" Tim Landscheidt
@ 2018-01-27 11:26 ` Ævar Arnfjörð Bjarmason
  2018-01-29 15:09 ` Johannes Schindelin
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
  5 siblings, 0 replies; 40+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-01-27 11:26 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git


On Fri, Jan 26 2018, Nguyễn Thái Ngọc Duy jotted:

> When a conflict happens during a rebase, you often need to look at the
> original patch to see what the changes are. This requires opening your
> favourite pager with some random path inside $GIT_DIR.
>
> This series makes that experience a bit better, by providing a command
> to read the patch. This is along the line of --edit-todo and --quit
> where you can just tell git what to do and not bother with details.
>
> My main focus is "git rebase", but because rebase uses "git am" behind
> the scene, "git am" gains --show-patch option too.
>
> There was something more I wanted to do, like coloring to the patch.
> But that probably will come later. I'll try to merge these two
> 21-months-old patches first.

This is only tangentially related to what you're doing, but I've long
wanted to add a commit.verbose config option to emulate `git commit
--verbose`, and furthermore to show the patch in rebase under "reword",
"squash" etc.

There's been so many times when I start editing the todo list, and
reword this or that, only to forget (because I don't have good commit
messages yet) what the patch is even about, and then switch to a
terminal, "git show" etc.

I'm just mentioning that here because if and when we have such a
feature, I think the --show-patch option is going to be very confusing,
people might want to enable this thing I'm talking about, but find that
--show-patch is something else entirely.

I don't know a good solution to that, just putting that out there.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
                   ` (3 preceding siblings ...)
  2018-01-27 11:26 ` Ævar Arnfjörð Bjarmason
@ 2018-01-29 15:09 ` Johannes Schindelin
  2018-01-30  9:10   ` Duy Nguyen
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
  5 siblings, 1 reply; 40+ messages in thread
From: Johannes Schindelin @ 2018-01-29 15:09 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

Hi Duy,

On Fri, 26 Jan 2018, Nguyễn Thái Ngọc Duy wrote:

> When a conflict happens during a rebase, you often need to look at the
> original patch to see what the changes are. This requires opening your
> favourite pager with some random path inside $GIT_DIR.
> 
> This series makes that experience a bit better, by providing a command
> to read the patch. This is along the line of --edit-todo and --quit
> where you can just tell git what to do and not bother with details.
> 
> My main focus is "git rebase", but because rebase uses "git am" behind
> the scene, "git am" gains --show-patch option too.

Makes sense. I am not a 100% certain that 2/2 catches all rebase -i corner
cases, but I think the patches are good enough even for `next` already.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-29 15:09 ` Johannes Schindelin
@ 2018-01-30  9:10   ` Duy Nguyen
  2018-01-30 12:32     ` Johannes Schindelin
  0 siblings, 1 reply; 40+ messages in thread
From: Duy Nguyen @ 2018-01-30  9:10 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List

On Mon, Jan 29, 2018 at 10:09 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Duy,
>
> On Fri, 26 Jan 2018, Nguyễn Thái Ngọc Duy wrote:
>
>> When a conflict happens during a rebase, you often need to look at the
>> original patch to see what the changes are. This requires opening your
>> favourite pager with some random path inside $GIT_DIR.
>>
>> This series makes that experience a bit better, by providing a command
>> to read the patch. This is along the line of --edit-todo and --quit
>> where you can just tell git what to do and not bother with details.
>>
>> My main focus is "git rebase", but because rebase uses "git am" behind
>> the scene, "git am" gains --show-patch option too.
>
> Makes sense. I am not a 100% certain that 2/2 catches all rebase -i corner
> cases, but I think the patches are good enough even for `next` already.

Not so fast :) With Tim's suggestion about using a pseudo ref and
AEvar complaint about potential confusion, I might actually go with
pseudo ref for rebase (and leave "git am" in the cold for now).
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 2/2] rebase: add --show-patch
  2018-01-26 11:22     ` Duy Nguyen
@ 2018-01-30 11:15       ` Phillip Wood
  0 siblings, 0 replies; 40+ messages in thread
From: Phillip Wood @ 2018-01-30 11:15 UTC (permalink / raw)
  To: Duy Nguyen, phillip.wood; +Cc: Git Mailing List

On 26/01/18 11:22, Duy Nguyen wrote:
> On Fri, Jan 26, 2018 at 6:12 PM, Phillip Wood <phillip.wood@talktalk.net> wrote:
>>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
>>> index 06a4723d4d..5c513a9736 100644
>>> --- a/git-rebase--merge.sh
>>> +++ b/git-rebase--merge.sh
>>> @@ -137,6 +137,10 @@ skip)
>>>       finish_rb_merge
>>>       return
>>>       ;;
>>> +show-patch)
>>> +     cmt="$(cat "$state_dir/current")"
>>> +     exec git format-patch --subject-prefix= --stdout "${cmt}^!"
>>> +     ;;
>>>  esac
>>
>> Here and in the git-rebase--interactive you have access to the SHA of
>> the failed pick so you could run git log --patch and git colored output
> 
> Yes. My first revision I actually did "git diff" here. The only
> problem is inconsistency because we can't color "git am --show-patch"
> the same way, the patch source is in text format, not in the repo. But
> if people are ok with that I sure would switch to "git show".
> 
>> and it would use the pager in the same way as 'git am --show-patch' does
> 
> format-patch does set up pager. If it does not I would be very
> annoyed. I added this for convenience after all.
> 
Ah, I didn't realize that (now I come to think of it I've only ever used
--stdout to redirect the output). As my perceived lack of pager was the
main reason I suggested using log I'd ignore me.

I think the suggestion of having a ref for 'rebase -i' and 'rebase -m'
could be good as it'd be more flexible though I'm not sure what you'd do
about plain old rebase.

Best Wishes

Phillip

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-30  9:10   ` Duy Nguyen
@ 2018-01-30 12:32     ` Johannes Schindelin
  2018-01-30 20:19       ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Johannes Schindelin @ 2018-01-30 12:32 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 1482 bytes --]

Hi Duy,

On Tue, 30 Jan 2018, Duy Nguyen wrote:

> On Mon, Jan 29, 2018 at 10:09 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Duy,
> >
> > On Fri, 26 Jan 2018, Nguyễn Thái Ngọc Duy wrote:
> >
> >> When a conflict happens during a rebase, you often need to look at the
> >> original patch to see what the changes are. This requires opening your
> >> favourite pager with some random path inside $GIT_DIR.
> >>
> >> This series makes that experience a bit better, by providing a command
> >> to read the patch. This is along the line of --edit-todo and --quit
> >> where you can just tell git what to do and not bother with details.
> >>
> >> My main focus is "git rebase", but because rebase uses "git am" behind
> >> the scene, "git am" gains --show-patch option too.
> >
> > Makes sense. I am not a 100% certain that 2/2 catches all rebase -i corner
> > cases, but I think the patches are good enough even for `next` already.
> 
> Not so fast :) With Tim's suggestion about using a pseudo ref and
> AEvar complaint about potential confusion, I might actually go with
> pseudo ref for rebase (and leave "git am" in the cold for now).

The pseudo ref certainly has an appeal. For people very familiar with
Git's peculiarities such as FETCH_HEAD. Such as myself.

For users, it is probably substantially worse an experience than having a
cmdmode like --show-patch in the very command they were just running.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-30 12:32     ` Johannes Schindelin
@ 2018-01-30 20:19       ` Junio C Hamano
  2018-02-01  2:06         ` Tim Landscheidt
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2018-01-30 20:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Duy Nguyen, Git Mailing List

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The pseudo ref certainly has an appeal. For people very familiar with
> Git's peculiarities such as FETCH_HEAD. Such as myself.
>
> For users, it is probably substantially worse an experience than having a
> cmdmode like --show-patch in the very command they were just running.

I tend to agree with that assessment.  FETCH_HEAD was a required
mechanism for commands in the toolchain to communicate and wasn't
meant as a mechanism for end-users.  I do not think it is a good 
idea to add to the pile to these special files the users would need
to look at, when we do not need to.  

Even if the internal implementation uses such a file, wrapping it
with a documented command mode would make a better UI.





^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2 0/3] Add "git rebase --show-patch"
  2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
                   ` (4 preceding siblings ...)
  2018-01-29 15:09 ` Johannes Schindelin
@ 2018-01-31  9:30 ` Nguyễn Thái Ngọc Duy
  2018-01-31  9:30   ` [PATCH v2 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
                     ` (4 more replies)
  5 siblings, 5 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-31  9:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim,
	Nguyễn Thái Ngọc Duy

v2:

- keeps --show-patch, but it's renamed to be more specific and
  hopefully less misleading. Other names I thought of were
  --show-patch-in-progress, --show-stopped-patch or
  --show-conflict-patch but they don't look as good.

  Also, since this is a long option name, I don't think length
  matters. You either go with tab completion, alias or short option
  name when you want to type it fast.

- --show-patch now uses git show instead of format-patch, this
  gives coloring for all rebase cases

- people who are not happy with --show-patch can roll their own with
  the help from the pseudo ref ORIG_COMMIT

Nguyễn Thái Ngọc Duy (3):
  am: add --show-current-patch
  rebase: add --show-current-patch
  rebase: introduce and use pseudo-ref ORIG_COMMIT

 Documentation/git-am.txt               |  6 +++-
 Documentation/git-rebase.txt           |  7 +++-
 builtin/am.c                           | 46 +++++++++++++++++++++++---
 contrib/completion/git-completion.bash |  8 ++---
 git-rebase--am.sh                      |  3 ++
 git-rebase--interactive.sh             |  6 ++++
 git-rebase--merge.sh                   |  5 +++
 git-rebase.sh                          |  8 ++++-
 sequencer.c                            |  3 ++
 t/t3400-rebase.sh                      | 34 +++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  8 +++++
 t/t4150-am.sh                          |  5 +++
 12 files changed, 128 insertions(+), 11 deletions(-)

-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2 1/3] am: add --show-current-patch
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
@ 2018-01-31  9:30   ` Nguyễn Thái Ngọc Duy
  2018-01-31  9:40     ` Eric Sunshine
  2018-01-31  9:30   ` [PATCH] gitignore.txt: elaborate shell glob syntax Nguyễn Thái Ngọc Duy
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-31  9:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim,
	Nguyễn Thái Ngọc Duy

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
around in there, which is not a good thing. With this, the user does
not have to keep the path around somewhere (because after a couple of
commands, the path may be out of scrollback buffer) when they need to
look at the patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  6 ++++-
 builtin/am.c                           | 31 ++++++++++++++++++++++----
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          |  5 +++++
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..0f426ae874 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-current-patch)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,10 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--show-current-patch::
+	Show the patch being applied when "git am" is stopped because
+	of conflicts.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..5eff1a648d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
 			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
 
 			if (advice_amworkdir)
-				printf_ln(_("The copy of the patch that failed is found in: %s"),
-						am_path(state, "patch"));
+				printf_ln(_("Use 'git am --show-current-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
@@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
 	am_destroy(state);
 }
 
+static int show_patch(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+	int len;
+
+	len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+	if (len < 0)
+		die_errno(_("failed to read '%s'"),
+			  am_path(state, msgnum(state)));
+
+	setup_pager();
+	write_in_full(1, sb.buf, sb.len);
+	strbuf_release(&sb);
+	return 0;
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2164,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2171,6 +2187,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	int patch_format = PATCH_FORMAT_UNKNOWN;
 	enum resume_mode resume = RESUME_FALSE;
 	int in_progress;
+	int ret = 0;
 
 	const char * const usage[] = {
 		N_("git am [<options>] [(<mbox> | <Maildir>)...]"),
@@ -2249,6 +2266,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "show-current-patch", &resume,
+			N_("show the patch being applied."),
+			RESUME_SHOW_PATCH),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2359,11 +2379,14 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_SHOW_PATCH:
+		ret = show_patch(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
 
 	am_state_release(&state);
 
-	return 0;
+	return ret;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..56ca445fa8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --show-current-patch"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..23abf42abc 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am pauses on conflict' '
 	test -d .git/rebase-apply
 '
 
+test_expect_success 'am --show-current-patch' '
+	git am --show-current-patch >actual.patch &&
+	test_cmp .git/rebase-apply/0001 actual.patch
+'
+
 test_expect_success 'am --skip works' '
 	echo goodbye >expected &&
 	git am --skip &&
-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH] gitignore.txt: elaborate shell glob syntax
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
  2018-01-31  9:30   ` [PATCH v2 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
@ 2018-01-31  9:30   ` Nguyễn Thái Ngọc Duy
  2018-01-31 23:22     ` Junio C Hamano
  2018-01-31  9:30   ` [PATCH v2 2/3] rebase: add --show-current-patch Nguyễn Thái Ngọc Duy
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-31  9:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim,
	Nguyễn Thái Ngọc Duy

`fnmatch(3)` is a great mention if the intended audience is
programmers. For normal users it's probably better to spell out what
a shell glob is.

This paragraph is updated to roughly tell what the main wildcards are
supposed to do. All the details are still hidden away behind the
`fnmatch(3)` wall because bringing the whole specification here may be
too much.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitignore.txt | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 63260f0056..0f4b1360bd 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -102,12 +102,11 @@ PATTERN FORMAT
    (relative to the toplevel of the work tree if not from a
    `.gitignore` file).
 
- - Otherwise, Git treats the pattern as a shell glob suitable
-   for consumption by fnmatch(3) with the FNM_PATHNAME flag:
-   wildcards in the pattern will not match a / in the pathname.
-   For example, "Documentation/{asterisk}.html" matches
-   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
-   or "tools/perf/Documentation/perf.html".
+ - Otherwise, Git treats the pattern as a shell glob: '{asterisk}'
+   matches anything except '/', '?' matches any one character except
+   '/' and '[]' matches one character in a selected range. See
+   fnmatch(3) and the FNM_PATHNAME flag for a more accurate
+   description.
 
  - A leading slash matches the beginning of the pathname.
    For example, "/{asterisk}.c" matches "cat-file.c" but not
-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2 2/3] rebase: add --show-current-patch
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
  2018-01-31  9:30   ` [PATCH v2 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
  2018-01-31  9:30   ` [PATCH] gitignore.txt: elaborate shell glob syntax Nguyễn Thái Ngọc Duy
@ 2018-01-31  9:30   ` Nguyễn Thái Ngọc Duy
  2018-01-31  9:30   ` [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT Nguyễn Thái Ngọc Duy
  2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
  4 siblings, 0 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-31  9:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim,
	Nguyễn Thái Ngọc Duy

It is useful to see the full patch while resolving conflicts in a
rebase. The only way to do it now is

    less .git/rebase-*/patch

which could turn out to be a lot longer to type if you are in a
linked worktree, or not at top-dir. On top of that, an ordinary user
should not need to peek into .git directory. The new option is
provided to examine the patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           |  6 ++++-
 builtin/am.c                           | 11 +++++++++
 contrib/completion/git-completion.bash |  4 ++--
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  3 +++
 git-rebase--merge.sh                   |  3 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 33 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  5 ++++
 9 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..7ef9577472 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream> [<branch>]]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort | --quit | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo | --show-current-patch
 
 DESCRIPTION
 -----------
@@ -250,6 +250,10 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --edit-todo::
 	Edit the todo list during an interactive rebase.
 
+--show-current-patch::
+	Show the current patch in an interactive rebase or when rebase
+	is stopped because of conflicts.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/builtin/am.c b/builtin/am.c
index 5eff1a648d..caec50cba9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2125,6 +2125,17 @@ static int show_patch(struct am_state *state)
 	struct strbuf sb = STRBUF_INIT;
 	int len;
 
+	if (!is_null_oid(&state->orig_commit)) {
+		const char *av[4] = { "show", NULL, "--", NULL };
+		char *new_oid_str;
+		int ret;
+
+		av[1] = new_oid_str = xstrdup(oid_to_hex(&state->orig_commit));
+		ret = run_command_v_opt(av, RUN_GIT_CMD);
+		free(new_oid_str);
+		return ret;
+	}
+
 	len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
 	if (len < 0)
 		die_errno(_("failed to read '%s'"),
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 56ca445fa8..2bd30d68cf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,11 +1992,11 @@ _git_rebase ()
 {
 	__git_find_repo_path
 	if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
-		__gitcomp "--continue --skip --abort --quit --edit-todo"
+		__gitcomp "--continue --skip --abort --quit --edit-todo --show-current-patch"
 		return
 	elif [ -d "$__git_repo_path"/rebase-apply ] || \
 	     [ -d "$__git_repo_path"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort --quit"
+		__gitcomp "--continue --skip --abort --quit --show-current-patch"
 		return
 	fi
 	__git_complete_strategy && return
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..c931891cbc 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-current-patch)
+	exec git am --show-current-patch
+	;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..0c0f8abbf9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -840,6 +840,9 @@ To continue rebase after editing, run:
 
 	exit
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/stopped-sha")" --
+	;;
 esac
 
 comment_for_reflog start
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..0a96dfae37 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,9 @@ skip)
 	finish_rb_merge
 	return
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/current")" --
+	;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..41c915d18c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
+show-current-patch! show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--quit|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo|--show-current-patch)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -412,6 +413,10 @@ quit)
 edit-todo)
 	run_specific_rebase
 	;;
+show-current-patch)
+	run_specific_rebase
+	die "BUG: run_specific_rebase is not supposed to return here"
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 8ac58d5ea5..09943d6a9b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -277,4 +277,37 @@ EOF
 	test_cmp From_.msg out
 '
 
+test_expect_success 'rebase--am.sh and --show-current-patch' '
+	test_create_repo conflict-apply &&
+	(
+		cd conflict-apply &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --onto init HEAD^ &&
+		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+		grep "show.*$(git rev-parse two)" stderr
+	)
+'
+
+test_expect_success 'rebase--merge.sh and --show-current-patch' '
+	test_create_repo conflict-merge &&
+	(
+		cd conflict-merge &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --merge --onto init HEAD^ &&
+		git rebase --show-current-patch >actual.patch &&
+		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+		grep "show.*$(git rev-parse two)" stderr
+	)
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 481a350090..3af6f149a9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -225,6 +225,11 @@ test_expect_success 'stop on conflicting pick' '
 	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
+test_expect_success 'show conflicted patch' '
+	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+	grep "show.*$(cat "$state_dir/stopped-sha")" stderr
+'
+
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
                     ` (2 preceding siblings ...)
  2018-01-31  9:30   ` [PATCH v2 2/3] rebase: add --show-current-patch Nguyễn Thái Ngọc Duy
@ 2018-01-31  9:30   ` Nguyễn Thái Ngọc Duy
  2018-01-31 23:18     ` Junio C Hamano
  2018-02-01 11:05     ` Phillip Wood
  2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
  4 siblings, 2 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-01-31  9:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim,
	Nguyễn Thái Ngọc Duy

The new command `git rebase --show-current-patch` is useful for seeing
the commit related to the current rebase state. Some however may find
the "git show" command behind it too limiting. You may want to
increase context lines, do a diff that ignores whitespaces...

For these advanced use cases, the user can execute any command they
want with the new pseudo ref ORIG_COMMIT.

This also helps show where the stopped commit is from, which is hard
to see from the previous patch which implements --show-current-patch.

Helped-by: Tim Landscheidt <tim@tim-landscheidt.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           | 3 ++-
 builtin/am.c                           | 4 ++++
 contrib/completion/git-completion.bash | 2 +-
 git-rebase--interactive.sh             | 5 ++++-
 git-rebase--merge.sh                   | 4 +++-
 git-rebase.sh                          | 1 +
 sequencer.c                            | 3 +++
 t/t3400-rebase.sh                      | 3 ++-
 t/t3404-rebase-interactive.sh          | 5 ++++-
 9 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7ef9577472..6da9296bf8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 
 --show-current-patch::
 	Show the current patch in an interactive rebase or when rebase
-	is stopped because of conflicts.
+	is stopped because of conflicts. This is the equivalent of
+	`git show ORIG_COMMIT`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index caec50cba9..bf9b356340 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir);
+	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 
 	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
 		am_destroy(state);
@@ -1110,6 +1111,7 @@ static void am_next(struct am_state *state)
 
 	oidclr(&state->orig_commit);
 	unlink(am_path(state, "original-commit"));
+	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 
 	if (!get_oid("HEAD", &head))
 		write_state_text(state, "abort-safety", oid_to_hex(&head));
@@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
 
 	oidcpy(&state->orig_commit, &commit_oid);
 	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
+	update_ref("am", "ORIG_COMMIT", &commit_oid,
+		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2bd30d68cf..deea688e0e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -439,7 +439,7 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
+			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c0f8abbf9..ef72bd5871 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,12 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref ORIG_COMMIT "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref ORIG_COMMIT "$1"
 	make_patch $1
 	git rev-parse --verify HEAD > "$amend"
 	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
@@ -841,7 +843,7 @@ To continue rebase after editing, run:
 	exit
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/stopped-sha")" --
+	exec git show ORIG_COMMIT --
 	;;
 esac
 
@@ -858,6 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
+rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 0a96dfae37..70966c32c3 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -57,6 +57,7 @@ call_merge () {
 	echo "$msgnum" >"$state_dir/msgnum"
 	cmt="$(cat "$state_dir/cmt.$msgnum")"
 	echo "$cmt" > "$state_dir/current"
+	git update-ref ORIG_COMMIT "$cmt"
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
@@ -138,13 +139,14 @@ skip)
 	return
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/current")" --
+	exec git show ORIG_COMMIT --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
+rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")
diff --git a/git-rebase.sh b/git-rebase.sh
index 41c915d18c..1db4301b90 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -182,6 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
 }
 
 finish_rebase () {
+	rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"
diff --git a/sequencer.c b/sequencer.c
index 4d3f60594c..fe907a0701 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1792,6 +1792,8 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
 	p = short_commit_name(commit);
 	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
 		return -1;
+	update_ref("rebase", "ORIG_COMMIT", &commit->object.oid,
+		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 
 	strbuf_addf(&buf, "%s/patch", get_dir(opts));
 	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -2043,6 +2045,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			unlink(rebase_path_author_script());
 			unlink(rebase_path_stopped_sha());
 			unlink(rebase_path_amend());
+			delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 09943d6a9b..2be4abcb7b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -306,7 +306,8 @@ test_expect_success 'rebase--merge.sh and --show-current-patch' '
 		test_must_fail git rebase --merge --onto init HEAD^ &&
 		git rebase --show-current-patch >actual.patch &&
 		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-		grep "show.*$(git rev-parse two)" stderr
+		grep "show.*ORIG_COMMIT" stderr &&
+		test "$(git rev-parse ORIG_COMMIT)" = "$(git rev-parse two)"
 	)
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3af6f149a9..c0fe0193bb 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -227,7 +227,10 @@ test_expect_success 'stop on conflicting pick' '
 
 test_expect_success 'show conflicted patch' '
 	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-	grep "show.*$(cat "$state_dir/stopped-sha")" stderr
+	grep "show.*ORIG_COMMIT" stderr &&
+	# the original stopped-sha1 is abbreviated
+	stopped_sha1="$(git rev-parse $(cat ".git/rebase-merge/stopped-sha"))" &&
+	test "$(git rev-parse ORIG_COMMIT)" = "$stopped_sha1"
 '
 
 test_expect_success 'abort' '
-- 
2.16.1.205.g271f633410


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 1/3] am: add --show-current-patch
  2018-01-31  9:30   ` [PATCH v2 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
@ 2018-01-31  9:40     ` Eric Sunshine
  2018-01-31 22:59       ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Eric Sunshine @ 2018-01-31  9:40 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: Git List, Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, tim

On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
> around in there, which is not a good thing. With this, the user does
> not have to keep the path around somewhere (because after a couple of
> commands, the path may be out of scrollback buffer) when they need to
> look at the patch.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/builtin/am.c b/builtin/am.c
> @@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
> +static int show_patch(struct am_state *state)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +       int len;
> +
> +       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
> +       if (len < 0)
> +               die_errno(_("failed to read '%s'"),
> +                         am_path(state, msgnum(state)));

Isn't this am_path() invocation inside die_errno() likely to clobber
the 'errno' from strbuf_read_file() which you want to be reporting?

> +       setup_pager();
> +       write_in_full(1, sb.buf, sb.len);
> +       strbuf_release(&sb);
> +       return 0;
> +}

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 1/3] am: add --show-current-patch
  2018-01-31  9:40     ` Eric Sunshine
@ 2018-01-31 22:59       ` Junio C Hamano
  2018-02-02  9:25         ` Duy Nguyen
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2018-01-31 22:59 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Nguyễn Thái Ngọc Duy, Git List,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin, tim

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
>> around in there, which is not a good thing. With this, the user does
>> not have to keep the path around somewhere (because after a couple of
>> commands, the path may be out of scrollback buffer) when they need to
>> look at the patch.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> diff --git a/builtin/am.c b/builtin/am.c
>> @@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
>> +static int show_patch(struct am_state *state)
>> +{
>> +       struct strbuf sb = STRBUF_INIT;
>> +       int len;
>> +
>> +       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
>> +       if (len < 0)
>> +               die_errno(_("failed to read '%s'"),
>> +                         am_path(state, msgnum(state)));
>
> Isn't this am_path() invocation inside die_errno() likely to clobber
> the 'errno' from strbuf_read_file() which you want to be reporting?

True.  After coming up with the pathname to the current patch file,
we are going to exit without ever calling am_path(), or underlying
get_pathname() via mkpath(), again before exiting anyway, so perhaps
it is sufficient to just do an am_path() just once upfront, feed it
to strbuf_read_file() and also to die_errno().

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
  2018-01-31  9:30   ` [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT Nguyễn Thái Ngọc Duy
@ 2018-01-31 23:18     ` Junio C Hamano
  2018-02-01 10:02       ` Duy Nguyen
  2018-02-01 11:05     ` Phillip Wood
  1 sibling, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2018-01-31 23:18 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> The new command `git rebase --show-current-patch` is useful for seeing
> the commit related to the current rebase state. Some however may find
> the "git show" command behind it too limiting. You may want to
> increase context lines, do a diff that ignores whitespaces...
>
> For these advanced use cases, the user can execute any command they
> want with the new pseudo ref ORIG_COMMIT.
>
> This also helps show where the stopped commit is from, which is hard
> to see from the previous patch which implements --show-current-patch.
>
> Helped-by: Tim Landscheidt <tim@tim-landscheidt.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Hmph, how is this new file conceptually different from existing ones
like CHERRY_PICK_HEAD?  

>  Documentation/git-rebase.txt           | 3 ++-
>  builtin/am.c                           | 4 ++++
>  contrib/completion/git-completion.bash | 2 +-
>  git-rebase--interactive.sh             | 5 ++++-
>  git-rebase--merge.sh                   | 4 +++-
>  git-rebase.sh                          | 1 +
>  sequencer.c                            | 3 +++
>  t/t3400-rebase.sh                      | 3 ++-
>  t/t3404-rebase-interactive.sh          | 5 ++++-
>  9 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 7ef9577472..6da9296bf8 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>  
>  --show-current-patch::
>  	Show the current patch in an interactive rebase or when rebase
> -	is stopped because of conflicts.
> +	is stopped because of conflicts. This is the equivalent of
> +	`git show ORIG_COMMIT`.
>  
>  -m::
>  --merge::
> diff --git a/builtin/am.c b/builtin/am.c
> index caec50cba9..bf9b356340 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
>  
>  	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
>  		die_errno(_("failed to create directory '%s'"), state->dir);
> +	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
>  
>  	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
>  		am_destroy(state);
> @@ -1110,6 +1111,7 @@ static void am_next(struct am_state *state)
>  
>  	oidclr(&state->orig_commit);
>  	unlink(am_path(state, "original-commit"));
> +	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
>  
>  	if (!get_oid("HEAD", &head))
>  		write_state_text(state, "abort-safety", oid_to_hex(&head));
> @@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
>  
>  	oidcpy(&state->orig_commit, &commit_oid);
>  	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
> +	update_ref("am", "ORIG_COMMIT", &commit_oid,
> +		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
>  
>  	return 0;
>  }
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2bd30d68cf..deea688e0e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -439,7 +439,7 @@ __git_refs ()
>  			track=""
>  			;;
>  		*)
> -			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
> +			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
>  				case "$i" in
>  				$match*)
>  					if [ -e "$dir/$i" ]; then
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0c0f8abbf9..ef72bd5871 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -199,12 +199,14 @@ make_patch () {
>  
>  die_with_patch () {
>  	echo "$1" > "$state_dir"/stopped-sha
> +	git update-ref ORIG_COMMIT "$1"
>  	make_patch "$1"
>  	die "$2"
>  }
>  
>  exit_with_patch () {
>  	echo "$1" > "$state_dir"/stopped-sha
> +	git update-ref ORIG_COMMIT "$1"
>  	make_patch $1
>  	git rev-parse --verify HEAD > "$amend"
>  	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
> @@ -841,7 +843,7 @@ To continue rebase after editing, run:
>  	exit
>  	;;
>  show-current-patch)
> -	exec git show "$(cat "$state_dir/stopped-sha")" --
> +	exec git show ORIG_COMMIT --
>  	;;
>  esac
>  
> @@ -858,6 +860,7 @@ fi
>  
>  orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
>  mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
> +rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
>  
>  : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
>  write_basic_state
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 0a96dfae37..70966c32c3 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -57,6 +57,7 @@ call_merge () {
>  	echo "$msgnum" >"$state_dir/msgnum"
>  	cmt="$(cat "$state_dir/cmt.$msgnum")"
>  	echo "$cmt" > "$state_dir/current"
> +	git update-ref ORIG_COMMIT "$cmt"
>  	hd=$(git rev-parse --verify HEAD)
>  	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
>  	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
> @@ -138,13 +139,14 @@ skip)
>  	return
>  	;;
>  show-current-patch)
> -	exec git show "$(cat "$state_dir/current")" --
> +	exec git show ORIG_COMMIT --
>  	;;
>  esac
>  
>  mkdir -p "$state_dir"
>  echo "$onto_name" > "$state_dir/onto_name"
>  write_basic_state
> +rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
>  
>  msgnum=0
>  for cmt in $(git rev-list --reverse --no-merges "$revisions")
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 41c915d18c..1db4301b90 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -182,6 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
>  }
>  
>  finish_rebase () {
> +	rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
>  	apply_autostash &&
>  	{ git gc --auto || true; } &&
>  	rm -rf "$state_dir"
> diff --git a/sequencer.c b/sequencer.c
> index 4d3f60594c..fe907a0701 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1792,6 +1792,8 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
>  	p = short_commit_name(commit);
>  	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
>  		return -1;
> +	update_ref("rebase", "ORIG_COMMIT", &commit->object.oid,
> +		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
>  
>  	strbuf_addf(&buf, "%s/patch", get_dir(opts));
>  	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
> @@ -2043,6 +2045,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
>  			unlink(rebase_path_author_script());
>  			unlink(rebase_path_stopped_sha());
>  			unlink(rebase_path_amend());
> +			delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
>  		}
>  		if (item->command <= TODO_SQUASH) {
>  			if (is_rebase_i(opts))
> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
> index 09943d6a9b..2be4abcb7b 100755
> --- a/t/t3400-rebase.sh
> +++ b/t/t3400-rebase.sh
> @@ -306,7 +306,8 @@ test_expect_success 'rebase--merge.sh and --show-current-patch' '
>  		test_must_fail git rebase --merge --onto init HEAD^ &&
>  		git rebase --show-current-patch >actual.patch &&
>  		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
> -		grep "show.*$(git rev-parse two)" stderr
> +		grep "show.*ORIG_COMMIT" stderr &&
> +		test "$(git rev-parse ORIG_COMMIT)" = "$(git rev-parse two)"
>  	)
>  '
>  
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 3af6f149a9..c0fe0193bb 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -227,7 +227,10 @@ test_expect_success 'stop on conflicting pick' '
>  
>  test_expect_success 'show conflicted patch' '
>  	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
> -	grep "show.*$(cat "$state_dir/stopped-sha")" stderr
> +	grep "show.*ORIG_COMMIT" stderr &&
> +	# the original stopped-sha1 is abbreviated
> +	stopped_sha1="$(git rev-parse $(cat ".git/rebase-merge/stopped-sha"))" &&
> +	test "$(git rev-parse ORIG_COMMIT)" = "$stopped_sha1"
>  '
>  
>  test_expect_success 'abort' '

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] gitignore.txt: elaborate shell glob syntax
  2018-01-31  9:30   ` [PATCH] gitignore.txt: elaborate shell glob syntax Nguyễn Thái Ngọc Duy
@ 2018-01-31 23:22     ` Junio C Hamano
  2018-02-01  9:59       ` Duy Nguyen
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2018-01-31 23:22 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

>     `.gitignore` file).
>  
> - - Otherwise, Git treats the pattern as a shell glob suitable
> -   for consumption by fnmatch(3) with the FNM_PATHNAME flag:
> -   wildcards in the pattern will not match a / in the pathname.
> -   For example, "Documentation/{asterisk}.html" matches
> -   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
> -   or "tools/perf/Documentation/perf.html".
> + - Otherwise, Git treats the pattern as a shell glob: '{asterisk}'
> +   matches anything except '/', '?' matches any one character except
> +   '/' and '[]' matches one character in a selected range. See
> +   fnmatch(3) and the FNM_PATHNAME flag for a more accurate
> +   description.

Where the original did not quote single letters at all, this uses a
pair of single quotes.  Did you make sure it renders well in HTML
and manpage already or should I do that for you before applying?

I think what you wrote is accurate enough already, and those who
want to go to fnmatch(3) would do so not for accuracy but for
authority ;-) Perhaps s/accurate/detailed/?




^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 0/2] Add "git rebase --show-patch"
  2018-01-30 20:19       ` Junio C Hamano
@ 2018-02-01  2:06         ` Tim Landscheidt
  0 siblings, 0 replies; 40+ messages in thread
From: Tim Landscheidt @ 2018-02-01  2:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Duy Nguyen, Git Mailing List

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

>> The pseudo ref certainly has an appeal. For people very familiar with
>> Git's peculiarities such as FETCH_HEAD. Such as myself.

>> For users, it is probably substantially worse an experience than having a
>> cmdmode like --show-patch in the very command they were just running.

> I tend to agree with that assessment.  FETCH_HEAD was a required
> mechanism for commands in the toolchain to communicate and wasn't
> meant as a mechanism for end-users.  I do not think it is a good
> idea to add to the pile to these special files the users would need
> to look at, when we do not need to.

> Even if the internal implementation uses such a file, wrapping it
> with a documented command mode would make a better UI.

I disagree with that as I had:

| [alias]
|         original-commit = !git show $(cat .git/rebase-apply/original-commit)

for a long time in my ~/.gitconfig :-) (until I realized
that Git accepted "rebase-apply/original-commit" everywhere;
for single-commit branches I always just used ORIG_HEAD).

This meant that whenever I wanted to look at the lay of the
land upon which the original commit was built, I had to do
"git original-commit" and copy & paste the SHA1 (without my
alias, I had to "git log ORIG_HEAD", pick hopefully the cor-
rect commit and copy & paste its SHA1).  Only with this SHA1
I could then run "git diff", "git show", "git grep", "git
blame", etc., etc., etc. because in my use cases looking at
the patch alone was usually not sufficient: I needed to know
why there was a conflict, i. e. how the file(s) the patch
changed looked before they had been altered upstream.

To me, this type of "algebra" is what makes Git so powerful:
I do not have to build a pipe that gets the SHA1 of a
branch's tip and xargs it to "git show", I can just say "git
show $branch".  Having a SHA1 with a special meaning that
has no easy way to access it by "git rev-parse" breaks this
UI IMHO.

Tim

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] gitignore.txt: elaborate shell glob syntax
  2018-01-31 23:22     ` Junio C Hamano
@ 2018-02-01  9:59       ` Duy Nguyen
  0 siblings, 0 replies; 40+ messages in thread
From: Duy Nguyen @ 2018-02-01  9:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim

On Wed, Jan 31, 2018 at 03:22:46PM -0800, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Argh.. stray patches strike again. It's not supposed to be in this
thread but in

https://public-inbox.org/git/%3C20180130101351.GA761@ash%3E/

> 
> >     `.gitignore` file).
> >  
> > - - Otherwise, Git treats the pattern as a shell glob suitable
> > -   for consumption by fnmatch(3) with the FNM_PATHNAME flag:
> > -   wildcards in the pattern will not match a / in the pathname.
> > -   For example, "Documentation/{asterisk}.html" matches
> > -   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
> > -   or "tools/perf/Documentation/perf.html".
> > + - Otherwise, Git treats the pattern as a shell glob: '{asterisk}'
> > +   matches anything except '/', '?' matches any one character except
> > +   '/' and '[]' matches one character in a selected range. See
> > +   fnmatch(3) and the FNM_PATHNAME flag for a more accurate
> > +   description.
> 
> Where the original did not quote single letters at all, this uses a
> pair of single quotes.  Did you make sure it renders well in HTML
> and manpage already or should I do that for you before applying?

I didn't. I thought I didn't add any weird symbols. I was wrong. These
are now wrapped as "`stuff`" to be displayed the same way as in nearby
paragraphs. Verified both man and HTML pages are rendered well.

> I think what you wrote is accurate enough already, and those who
> want to go to fnmatch(3) would do so not for accuracy but for
> authority ;-) Perhaps s/accurate/detailed/?

Well there are rooms for guessing, for example "matches anything" does
not tell you straight that it can match multiple characters. Anyway
fixed too.

-- 8< --
Subject: [PATCH v2] gitignore.txt: elaborate shell glob syntax

`fnmatch(3)` is a great mention if the intended audience is
programmers. For normal users it's probably better to spell out what
a shell glob is.

This paragraph is updated to roughly tell (or remind) what the main
wildcards are supposed to do. All the details are still hidden away
behind the `fnmatch(3)` wall because bringing the whole specification
here may be too much.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitignore.txt | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 63260f0056..ff5d7f9ed6 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -102,12 +102,11 @@ PATTERN FORMAT
    (relative to the toplevel of the work tree if not from a
    `.gitignore` file).
 
- - Otherwise, Git treats the pattern as a shell glob suitable
-   for consumption by fnmatch(3) with the FNM_PATHNAME flag:
-   wildcards in the pattern will not match a / in the pathname.
-   For example, "Documentation/{asterisk}.html" matches
-   "Documentation/git.html" but not "Documentation/ppc/ppc.html"
-   or "tools/perf/Documentation/perf.html".
+ - Otherwise, Git treats the pattern as a shell glob: "`*`" matches
+   anything except "`/`", "`?`" matches any one character except "`/`"
+   and "`[]`" matches one character in a selected range. See
+   fnmatch(3) and the FNM_PATHNAME flag for a more detailed
+   description.
 
  - A leading slash matches the beginning of the pathname.
    For example, "/{asterisk}.c" matches "cat-file.c" but not
-- 
2.16.1.205.g271f633410

-- 8< --

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
  2018-01-31 23:18     ` Junio C Hamano
@ 2018-02-01 10:02       ` Duy Nguyen
  2018-02-02 19:00         ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Duy Nguyen @ 2018-02-01 10:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, Tim Landscheidt

On Thu, Feb 1, 2018 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> The new command `git rebase --show-current-patch` is useful for seeing
>> the commit related to the current rebase state. Some however may find
>> the "git show" command behind it too limiting. You may want to
>> increase context lines, do a diff that ignores whitespaces...
>>
>> For these advanced use cases, the user can execute any command they
>> want with the new pseudo ref ORIG_COMMIT.
>>
>> This also helps show where the stopped commit is from, which is hard
>> to see from the previous patch which implements --show-current-patch.
>>
>> Helped-by: Tim Landscheidt <tim@tim-landscheidt.de>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>
> Hmph, how is this new file conceptually different from existing ones
> like CHERRY_PICK_HEAD?

Conceptually the same, except that CHERRY_PICK_HEAD can't be reused
because it's specifically tied to git-cherry-pick (there's even code
that delete this ref if cherry-pick is run as part of rebase, and
git-status uses this ref to see if a cherry-pick is in progress).
There's also REVERT_HEAD in sequencer.c, same purpose but for
git-revert. Perhaps I should rename this new ref to REBASE_HEAD to
follow the same naming?
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
  2018-01-31  9:30   ` [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT Nguyễn Thái Ngọc Duy
  2018-01-31 23:18     ` Junio C Hamano
@ 2018-02-01 11:05     ` Phillip Wood
  1 sibling, 0 replies; 40+ messages in thread
From: Phillip Wood @ 2018-02-01 11:05 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy, git
  Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim

On 31/01/18 09:30, Nguyễn Thái Ngọc Duy wrote:
> 
> The new command `git rebase --show-current-patch` is useful for seeing
> the commit related to the current rebase state. Some however may find
> the "git show" command behind it too limiting. You may want to
> increase context lines, do a diff that ignores whitespaces...
> 
> For these advanced use cases, the user can execute any command they
> want with the new pseudo ref ORIG_COMMIT.
> 
> This also helps show where the stopped commit is from, which is hard
> to see from the previous patch which implements --show-current-patch.
> 
> Helped-by: Tim Landscheidt <tim@tim-landscheidt.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Documentation/git-rebase.txt           | 3 ++-
>  builtin/am.c                           | 4 ++++
>  contrib/completion/git-completion.bash | 2 +-
>  git-rebase--interactive.sh             | 5 ++++-
>  git-rebase--merge.sh                   | 4 +++-
>  git-rebase.sh                          | 1 +
>  sequencer.c                            | 3 +++
>  t/t3400-rebase.sh                      | 3 ++-
>  t/t3404-rebase-interactive.sh          | 5 ++++-
>  9 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 7ef9577472..6da9296bf8 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults to HEAD.
>  
>  --show-current-patch::
>  	Show the current patch in an interactive rebase or when rebase
> -	is stopped because of conflicts.
> +	is stopped because of conflicts. This is the equivalent of
> +	`git show ORIG_COMMIT`.
>  
>  -m::
>  --merge::
> diff --git a/builtin/am.c b/builtin/am.c
> index caec50cba9..bf9b356340 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
>  
>  	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
>  		die_errno(_("failed to create directory '%s'"), state->dir);
> +	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
>  
>  	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
>  		am_destroy(state);
> @@ -1110,6 +1111,7 @@ static void am_next(struct am_state *state)
>  
>  	oidclr(&state->orig_commit);
>  	unlink(am_path(state, "original-commit"));
> +	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
>  
>  	if (!get_oid("HEAD", &head))
>  		write_state_text(state, "abort-safety", oid_to_hex(&head));
> @@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
>  
>  	oidcpy(&state->orig_commit, &commit_oid);
>  	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
> +	update_ref("am", "ORIG_COMMIT", &commit_oid,
> +		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
>  
>  	return 0;
>  }
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2bd30d68cf..deea688e0e 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -439,7 +439,7 @@ __git_refs ()
>  			track=""
>  			;;
>  		*)
> -			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
> +			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
>  				case "$i" in
>  				$match*)
>  					if [ -e "$dir/$i" ]; then
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0c0f8abbf9..ef72bd5871 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -199,12 +199,14 @@ make_patch () {
>  
>  die_with_patch () {
>  	echo "$1" > "$state_dir"/stopped-sha
> +	git update-ref ORIG_COMMIT "$1"
>  	make_patch "$1"
>  	die "$2"
>  }
>  
>  exit_with_patch () {
>  	echo "$1" > "$state_dir"/stopped-sha
> +	git update-ref ORIG_COMMIT "$1"
>  	make_patch $1
>  	git rev-parse --verify HEAD > "$amend"
>  	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
> @@ -841,7 +843,7 @@ To continue rebase after editing, run:
>  	exit
>  	;;
>  show-current-patch)
> -	exec git show "$(cat "$state_dir/stopped-sha")" --
> +	exec git show ORIG_COMMIT --
>  	;;
>  esac
>  
> @@ -858,6 +860,7 @@ fi
>  
>  orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
>  mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
> +rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
>  
>  : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
>  write_basic_state
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 0a96dfae37..70966c32c3 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -57,6 +57,7 @@ call_merge () {
>  	echo "$msgnum" >"$state_dir/msgnum"
>  	cmt="$(cat "$state_dir/cmt.$msgnum")"
>  	echo "$cmt" > "$state_dir/current"
> +	git update-ref ORIG_COMMIT "$cmt"
>  	hd=$(git rev-parse --verify HEAD)
>  	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
>  	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
> @@ -138,13 +139,14 @@ skip)
>  	return
>  	;;
>  show-current-patch)
> -	exec git show "$(cat "$state_dir/current")" --
> +	exec git show ORIG_COMMIT --
>  	;;
>  esac
>  
>  mkdir -p "$state_dir"
>  echo "$onto_name" > "$state_dir/onto_name"
>  write_basic_state
> +rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
>  
>  msgnum=0
>  for cmt in $(git rev-list --reverse --no-merges "$revisions")
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 41c915d18c..1db4301b90 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -182,6 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
>  }
>  
>  finish_rebase () {
> +	rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
>  	apply_autostash &&
>  	{ git gc --auto || true; } &&
>  	rm -rf "$state_dir"
> diff --git a/sequencer.c b/sequencer.c
> index 4d3f60594c..fe907a0701 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1792,6 +1792,8 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
>  	p = short_commit_name(commit);
>  	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
>  		return -1;
> +	update_ref("rebase", "ORIG_COMMIT", &commit->object.oid,
> +		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);

The sequencer tries to return errors rather than dying so the caller can
handle them. The other callers of update_ref pass UPDATE_REFS_MSG_ON_ERR

Best Wishes

Phillip

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 1/3] am: add --show-current-patch
  2018-01-31 22:59       ` Junio C Hamano
@ 2018-02-02  9:25         ` Duy Nguyen
  2018-02-02  9:46           ` Eric Sunshine
  2018-02-02 18:56           ` Junio C Hamano
  0 siblings, 2 replies; 40+ messages in thread
From: Duy Nguyen @ 2018-02-02  9:25 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Sunshine, Git List, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, tim

On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> >> Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
> >> around in there, which is not a good thing. With this, the user does
> >> not have to keep the path around somewhere (because after a couple of
> >> commands, the path may be out of scrollback buffer) when they need to
> >> look at the patch.
> >>
> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> >> ---
> >> diff --git a/builtin/am.c b/builtin/am.c
> >> @@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
> >> +static int show_patch(struct am_state *state)
> >> +{
> >> +       struct strbuf sb = STRBUF_INIT;
> >> +       int len;
> >> +
> >> +       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
> >> +       if (len < 0)
> >> +               die_errno(_("failed to read '%s'"),
> >> +                         am_path(state, msgnum(state)));
> >
> > Isn't this am_path() invocation inside die_errno() likely to clobber
> > the 'errno' from strbuf_read_file() which you want to be reporting?
> 
> True.

Thanks both. Good catch. Of course I will fix this in the re-roll, but
should we also do something for the current code base with the
following patch?

I think this is something coccinelle can help catch, but my
coccinelle-foo is way too low to come up with something like
"die_errno() must not take any arguments as function calls, except _()
and N_()".

-- 8< --
Subject: [PATCH] Preserve errno in case case before calling sth_errno()

All these locations do something like this

    sth_errno(..., somefunc(...));

where somefunc() can potentially change errno, which will be read by
sth_errno(). Call somefunc separately with errno preserved to avoid
this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/am.c      |  8 +++++++-
 builtin/commit.c  |  8 ++++++--
 builtin/init-db.c |  9 ++++++---
 rerere.c          |  9 ++++++---
 shallow.c         | 27 ++++++++++++++++++---------
 5 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..ba67e187a4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -244,6 +244,9 @@ static int am_in_progress(const struct am_state *state)
 static int read_state_file(struct strbuf *sb, const struct am_state *state,
 			const char *file, int trim)
 {
+	int saved_errno;
+	const char *path;
+
 	strbuf_reset(sb);
 
 	if (strbuf_read_file(sb, am_path(state, file), 0) >= 0) {
@@ -256,7 +259,10 @@ static int read_state_file(struct strbuf *sb, const struct am_state *state,
 	if (errno == ENOENT)
 		return -1;
 
-	die_errno(_("could not read '%s'"), am_path(state, file));
+	saved_errno = errno;
+	path = am_path(state, file);
+	errno = saved_errno;
+	die_errno(_("could not read '%s'"), path);
 }
 
 /**
diff --git a/builtin/commit.c b/builtin/commit.c
index 4610e3d8e3..39836b3734 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -780,8 +780,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	}
 
 	s->fp = fopen_for_writing(git_path_commit_editmsg());
-	if (s->fp == NULL)
-		die_errno(_("could not open '%s'"), git_path_commit_editmsg());
+	if (s->fp == NULL) {
+		int saved_errno = errno;
+		const char *path = git_path_commit_editmsg();
+		errno = saved_errno;
+		die_errno(_("could not open '%s'"), path);
+	}
 
 	/* Ignore status.displayCommentPrefix: we do need comments in COMMIT_EDITMSG. */
 	old_display_comment_prefix = s->display_comment_prefix;
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c9b7946bad..e384749f73 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -570,9 +570,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 			set_git_work_tree(work_tree);
 		else
 			set_git_work_tree(git_work_tree_cfg);
-		if (access(get_git_work_tree(), X_OK))
-			die_errno (_("Cannot access work tree '%s'"),
-				   get_git_work_tree());
+		if (access(get_git_work_tree(), X_OK)) {
+			int saved_errno = errno;
+			const char *path = get_git_work_tree();
+			errno = saved_errno;
+			die_errno(_("Cannot access work tree '%s'"), path);
+		}
 	}
 	else {
 		if (work_tree)
diff --git a/rerere.c b/rerere.c
index 1ce440f4bb..f19a53ff2c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -683,9 +683,12 @@ static int merge(const struct rerere_id *id, const char *path)
 	 * A successful replay of recorded resolution.
 	 * Mark that "postimage" was used to help gc.
 	 */
-	if (utime(rerere_path(id, "postimage"), NULL) < 0)
-		warning_errno("failed utime() on %s",
-			      rerere_path(id, "postimage"));
+	if (utime(rerere_path(id, "postimage"), NULL) < 0) {
+		int saved_errno = errno;
+		const char *path = rerere_path(id, "postimage");
+		errno = saved_errno;
+		warning_errno("failed utime() on %s", path);
+	}
 
 	/* Update "path" with the resolution */
 	f = fopen(path, "w");
diff --git a/shallow.c b/shallow.c
index df4d44ea7a..9da82f5292 100644
--- a/shallow.c
+++ b/shallow.c
@@ -295,9 +295,12 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
 		temp = xmks_tempfile(git_path("shallow_XXXXXX"));
 
 		if (write_in_full(temp->fd, sb.buf, sb.len) < 0 ||
-		    close_tempfile_gently(temp) < 0)
-			die_errno("failed to write to %s",
-				  get_tempfile_path(temp));
+		    close_tempfile_gently(temp) < 0) {
+			int saved_errno = errno;
+			const char *path = get_tempfile_path(temp);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		strbuf_release(&sb);
 		return get_tempfile_path(temp);
 	}
@@ -319,9 +322,12 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,
 				       LOCK_DIE_ON_ERROR);
 	check_shallow_file_for_update();
 	if (write_shallow_commits(&sb, 0, extra)) {
-		if (write_in_full(fd, sb.buf, sb.len) < 0)
-			die_errno("failed to write to %s",
-				  get_lock_file_path(shallow_lock));
+		if (write_in_full(fd, sb.buf, sb.len) < 0) {
+			int saved_errno = errno;
+			const char *path = get_lock_file_path(shallow_lock);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		*alternate_shallow_file = get_lock_file_path(shallow_lock);
 	} else
 		/*
@@ -366,9 +372,12 @@ void prune_shallow(int show_only)
 				       LOCK_DIE_ON_ERROR);
 	check_shallow_file_for_update();
 	if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {
-		if (write_in_full(fd, sb.buf, sb.len) < 0)
-			die_errno("failed to write to %s",
-				  get_lock_file_path(&shallow_lock));
+		if (write_in_full(fd, sb.buf, sb.len) < 0) {
+			int saved_errno = errno;
+			const char *path = get_lock_file_path(&shallow_lock);
+			errno = saved_errno;
+			die_errno("failed to write to %s", path);
+		}
 		commit_lock_file(&shallow_lock);
 	} else {
 		unlink(git_path_shallow());
-- 
2.16.1.205.g271f633410

-- 8< --

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 1/3] am: add --show-current-patch
  2018-02-02  9:25         ` Duy Nguyen
@ 2018-02-02  9:46           ` Eric Sunshine
  2018-02-02  9:53             ` Duy Nguyen
  2018-02-02 18:56           ` Junio C Hamano
  1 sibling, 1 reply; 40+ messages in thread
From: Eric Sunshine @ 2018-02-02  9:46 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Junio C Hamano, Git List, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Tim Landscheidt

On Fri, Feb 2, 2018 at 4:25 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> >> +       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
>> >> +       if (len < 0)
>> >> +               die_errno(_("failed to read '%s'"),
>> >> +                         am_path(state, msgnum(state)));
>> >
>> > Isn't this am_path() invocation inside die_errno() likely to clobber
>> > the 'errno' from strbuf_read_file() which you want to be reporting?
>> True.
>
> Thanks both. Good catch. Of course I will fix this in the re-roll, but
> should we also do something for the current code base with the
> following patch?
>
> -       die_errno(_("could not read '%s'"), am_path(state, file));
> +       saved_errno = errno;
> +       path = am_path(state, file);
> +       errno = saved_errno;
> +       die_errno(_("could not read '%s'"), path);

Rather than worrying about catching these at review time, I had been
thinking about a solution which automates it using variadic macros.
Something like:

    #define die_errno(...) do { \
        int saved_errno_ = errno; \
        die_errno_(saved_errno_, __VA_ARGS__); \
        } while (0);

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 1/3] am: add --show-current-patch
  2018-02-02  9:46           ` Eric Sunshine
@ 2018-02-02  9:53             ` Duy Nguyen
  0 siblings, 0 replies; 40+ messages in thread
From: Duy Nguyen @ 2018-02-02  9:53 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Junio C Hamano, Git List, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Tim Landscheidt

On Fri, Feb 2, 2018 at 4:46 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Fri, Feb 2, 2018 at 4:25 AM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, Jan 31, 2018 at 02:59:32PM -0800, Junio C Hamano wrote:
>>> Eric Sunshine <sunshine@sunshineco.com> writes:
>>> > On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>>> >> +       len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
>>> >> +       if (len < 0)
>>> >> +               die_errno(_("failed to read '%s'"),
>>> >> +                         am_path(state, msgnum(state)));
>>> >
>>> > Isn't this am_path() invocation inside die_errno() likely to clobber
>>> > the 'errno' from strbuf_read_file() which you want to be reporting?
>>> True.
>>
>> Thanks both. Good catch. Of course I will fix this in the re-roll, but
>> should we also do something for the current code base with the
>> following patch?
>>
>> -       die_errno(_("could not read '%s'"), am_path(state, file));
>> +       saved_errno = errno;
>> +       path = am_path(state, file);
>> +       errno = saved_errno;
>> +       die_errno(_("could not read '%s'"), path);
>
> Rather than worrying about catching these at review time, I had been
> thinking about a solution which automates it using variadic macros.
> Something like:
>
>     #define die_errno(...) do { \
>         int saved_errno_ = errno; \
>         die_errno_(saved_errno_, __VA_ARGS__); \
>         } while (0);

That would be best. Unfortunately we have HAVE_VARIADIC_MACROS so we
need to deal with no variadic macro support too.
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 1/3] am: add --show-current-patch
  2018-02-02  9:25         ` Duy Nguyen
  2018-02-02  9:46           ` Eric Sunshine
@ 2018-02-02 18:56           ` Junio C Hamano
  1 sibling, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2018-02-02 18:56 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Eric Sunshine, Git List, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, tim

Duy Nguyen <pclouds@gmail.com> writes:

> Subject: [PATCH] Preserve errno in case case before calling sth_errno()
>
> All these locations do something like this
>
>     sth_errno(..., somefunc(...));
>
> where somefunc() can potentially change errno, which will be read by
> sth_errno(). Call somefunc separately with errno preserved to avoid
> this.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/am.c      |  8 +++++++-
>  builtin/commit.c  |  8 ++++++--
>  builtin/init-db.c |  9 ++++++---
>  rerere.c          |  9 ++++++---
>  shallow.c         | 27 ++++++++++++++++++---------
>  5 files changed, 43 insertions(+), 18 deletions(-)

> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index c9b7946bad..e384749f73 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -570,9 +570,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  			set_git_work_tree(work_tree);
>  		else
>  			set_git_work_tree(git_work_tree_cfg);
> -		if (access(get_git_work_tree(), X_OK))
> -			die_errno (_("Cannot access work tree '%s'"),
> -				   get_git_work_tree());
> +		if (access(get_git_work_tree(), X_OK)) {
> +			int saved_errno = errno;
> +			const char *path = get_git_work_tree();
> +			errno = saved_errno;
> +			die_errno(_("Cannot access work tree '%s'"), path);
> +		}
>  	}

This one is the most faithful conversion from "mechanical rewrite"
point of view, but I wonder if we should instead take the returned
path from get_git_work_tree() and use it in both calls.  After all,
this is hardly performance sensitive codepath, so even "an obviously
safe but wasteful with extra xstrdup/free" version

	work_tree_path = xstrdup(get_git_work_tree());
	if (access(work_tree_path, X_OK))
		die_errno(_("msg..."), work_tree_path);
	free(work_tree_path);

may be an improvement.

> diff --git a/rerere.c b/rerere.c
> index 1ce440f4bb..f19a53ff2c 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -683,9 +683,12 @@ static int merge(const struct rerere_id *id, const char *path)
>  	 * A successful replay of recorded resolution.
>  	 * Mark that "postimage" was used to help gc.
>  	 */
> -	if (utime(rerere_path(id, "postimage"), NULL) < 0)
> -		warning_errno("failed utime() on %s",
> -			      rerere_path(id, "postimage"));
> +	if (utime(rerere_path(id, "postimage"), NULL) < 0) {
> +		int saved_errno = errno;
> +		const char *path = rerere_path(id, "postimage");
> +		errno = saved_errno;
> +		warning_errno("failed utime() on %s", path);
> +	}

Likewise.

> diff --git a/shallow.c b/shallow.c
> index df4d44ea7a..9da82f5292 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -295,9 +295,12 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
>  		temp = xmks_tempfile(git_path("shallow_XXXXXX"));
>  
>  		if (write_in_full(temp->fd, sb.buf, sb.len) < 0 ||
> -		    close_tempfile_gently(temp) < 0)
> -			die_errno("failed to write to %s",
> -				  get_tempfile_path(temp));
> +		    close_tempfile_gently(temp) < 0) {
> +			int saved_errno = errno;
> +			const char *path = get_tempfile_path(temp);
> +			errno = saved_errno;
> +			die_errno("failed to write to %s", path);
> +		}

It feels a bit questionable to my taste to pretend that we are truly
oblivious to how trivial get_tempfile_path() is, i.e. no more than
just a few field accesses to "tempfile" struct.  It buries more
important thing that is happening in the code in noise.

> @@ -319,9 +322,12 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,

Likewise.

> @@ -366,9 +372,12 @@ void prune_shallow(int show_only)

Likewise.

All others I snipped looked like good changes.  Thanks.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
  2018-02-01 10:02       ` Duy Nguyen
@ 2018-02-02 19:00         ` Junio C Hamano
  0 siblings, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2018-02-02 19:00 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Git Mailing List, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, Tim Landscheidt

Duy Nguyen <pclouds@gmail.com> writes:

> On Thu, Feb 1, 2018 at 6:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> Hmph, how is this new file conceptually different from existing ones
>> like CHERRY_PICK_HEAD?
>
> Conceptually the same, except that CHERRY_PICK_HEAD can't be reused
> because it's specifically tied to git-cherry-pick (there's even code
> that delete this ref if cherry-pick is run as part of rebase, and
> git-status uses this ref to see if a cherry-pick is in progress).
> There's also REVERT_HEAD in sequencer.c, same purpose but for
> git-revert. Perhaps I should rename this new ref to REBASE_HEAD to
> follow the same naming?

I just found "ORIG_COMMIT" too similar to "ORIG_HEAD" that is
totally a different thing and feared unnecessary confusion.  I think
you are correct to suggest that REBASE_HEAD would be more in line
with the naming convention with the sequencer-like operations.

Thanks.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v3 0/3] Add "git rebase --show-current-patch"
  2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
                     ` (3 preceding siblings ...)
  2018-01-31  9:30   ` [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT Nguyễn Thái Ngọc Duy
@ 2018-02-11  9:43   ` Nguyễn Thái Ngọc Duy
  2018-02-11  9:43     ` [PATCH v3 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
                       ` (3 more replies)
  4 siblings, 4 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-02-11  9:43 UTC (permalink / raw)
  To: git
  Cc: phillip.wood, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim, Nguyễn Thái Ngọc Duy

Compared to v2:

- the potential loss of errno before it's printed out in builtin/am.c
  is fixed.
- keep update_ref() in sequencer.c non-fatal like this rest
- rename ORIG_COMMIT to REBASE_HEAD

Interdiff:

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 6da9296bf8..0b29e48221 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -253,7 +253,7 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --show-current-patch::
 	Show the current patch in an interactive rebase or when rebase
 	is stopped because of conflicts. This is the equivalent of
-	`git show ORIG_COMMIT`.
+	`git show REBASE_HEAD`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index bf9b356340..21aedec41f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1011,7 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir);
-	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
+	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
 	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
 		am_destroy(state);
@@ -1111,7 +1111,7 @@ static void am_next(struct am_state *state)
 
 	oidclr(&state->orig_commit);
 	unlink(am_path(state, "original-commit"));
-	delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
+	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
 	if (!get_oid("HEAD", &head))
 		write_state_text(state, "abort-safety", oid_to_hex(&head));
@@ -1443,8 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
 
 	oidcpy(&state->orig_commit, &commit_oid);
 	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
-	update_ref("am", "ORIG_COMMIT", &commit_oid,
-		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	update_ref("am", "REBASE_HEAD", &commit_oid,
+		   NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }
@@ -2127,6 +2127,7 @@ static void am_abort(struct am_state *state)
 static int show_patch(struct am_state *state)
 {
 	struct strbuf sb = STRBUF_INIT;
+	const char *patch_path;
 	int len;
 
 	if (!is_null_oid(&state->orig_commit)) {
@@ -2140,10 +2141,10 @@ static int show_patch(struct am_state *state)
 		return ret;
 	}
 
-	len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
+	patch_path = am_path(state, msgnum(state));
+	len = strbuf_read_file(&sb, patch_path, 0);
 	if (len < 0)
-		die_errno(_("failed to read '%s'"),
-			  am_path(state, msgnum(state)));
+		die_errno(_("failed to read '%s'"), patch_path);
 
 	setup_pager();
 	write_in_full(1, sb.buf, sb.len);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index deea688e0e..8777805c9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -439,7 +439,7 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD ORIG_COMMIT; do
+			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD REBASE_HEAD; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ef72bd5871..a613156bcb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,14 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
-	git update-ref ORIG_COMMIT "$1"
+	git update-ref REBASE_HEAD "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
-	git update-ref ORIG_COMMIT "$1"
+	git update-ref REBASE_HEAD "$1"
 	make_patch $1
 	git rev-parse --verify HEAD > "$amend"
 	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
@@ -843,7 +843,7 @@ To continue rebase after editing, run:
 	exit
 	;;
 show-current-patch)
-	exec git show ORIG_COMMIT --
+	exec git show REBASE_HEAD --
 	;;
 esac
 
@@ -860,7 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
-rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 70966c32c3..957688f236 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -57,7 +57,7 @@ call_merge () {
 	echo "$msgnum" >"$state_dir/msgnum"
 	cmt="$(cat "$state_dir/cmt.$msgnum")"
 	echo "$cmt" > "$state_dir/current"
-	git update-ref ORIG_COMMIT "$cmt"
+	git update-ref REBASE_HEAD "$cmt"
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
@@ -139,14 +139,14 @@ skip)
 	return
 	;;
 show-current-patch)
-	exec git show ORIG_COMMIT --
+	exec git show REBASE_HEAD --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
-rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")
diff --git a/git-rebase.sh b/git-rebase.sh
index 1db4301b90..a13a581fe6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -182,7 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
 }
 
 finish_rebase () {
-	rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
+	rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"
diff --git a/sequencer.c b/sequencer.c
index fe907a0701..f692221999 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1792,8 +1792,9 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
 	p = short_commit_name(commit);
 	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
 		return -1;
-	update_ref("rebase", "ORIG_COMMIT", &commit->object.oid,
-		   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+	if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
+		       NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
+		res |= error(_("could not update %s"), "REBASE_HEAD");
 
 	strbuf_addf(&buf, "%s/patch", get_dir(opts));
 	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -2045,7 +2046,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			unlink(rebase_path_author_script());
 			unlink(rebase_path_stopped_sha());
 			unlink(rebase_path_amend());
-			delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
+			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 2be4abcb7b..72d9564747 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -306,8 +306,8 @@ test_expect_success 'rebase--merge.sh and --show-current-patch' '
 		test_must_fail git rebase --merge --onto init HEAD^ &&
 		git rebase --show-current-patch >actual.patch &&
 		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-		grep "show.*ORIG_COMMIT" stderr &&
-		test "$(git rev-parse ORIG_COMMIT)" = "$(git rev-parse two)"
+		grep "show.*REBASE_HEAD" stderr &&
+		test "$(git rev-parse REBASE_HEAD)" = "$(git rev-parse two)"
 	)
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index c0fe0193bb..23a54a4c49 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -227,10 +227,10 @@ test_expect_success 'stop on conflicting pick' '
 
 test_expect_success 'show conflicted patch' '
 	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-	grep "show.*ORIG_COMMIT" stderr &&
+	grep "show.*REBASE_HEAD" stderr &&
 	# the original stopped-sha1 is abbreviated
 	stopped_sha1="$(git rev-parse $(cat ".git/rebase-merge/stopped-sha"))" &&
-	test "$(git rev-parse ORIG_COMMIT)" = "$stopped_sha1"
+	test "$(git rev-parse REBASE_HEAD)" = "$stopped_sha1"
 '
 
 test_expect_success 'abort' '
-- 
2.16.1.399.g632f88eed1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v3 1/3] am: add --show-current-patch
  2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
@ 2018-02-11  9:43     ` Nguyễn Thái Ngọc Duy
  2018-02-11  9:43     ` [PATCH v3 2/3] rebase: " Nguyễn Thái Ngọc Duy
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-02-11  9:43 UTC (permalink / raw)
  To: git
  Cc: phillip.wood, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim, Nguyễn Thái Ngọc Duy

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
around in there, which is not a good thing. With this, the user does
not have to keep the path around somewhere (because after a couple of
commands, the path may be out of scrollback buffer) when they need to
look at the patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-am.txt               |  6 ++++-
 builtin/am.c                           | 32 ++++++++++++++++++++++----
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh                          |  5 ++++
 4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..0f426ae874 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 	 [--exclude=<path>] [--include=<path>] [--reject] [-q | --quiet]
 	 [--[no-]scissors] [-S[<keyid>]] [--patch-format=<format>]
 	 [(<mbox> | <Maildir>)...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-current-patch)
 
 DESCRIPTION
 -----------
@@ -167,6 +167,10 @@ default.   You can use `--no-utf8` to override this.
 --abort::
 	Restore the original branch and abort the patching operation.
 
+--show-current-patch::
+	Show the patch being applied when "git am" is stopped because
+	of conflicts.
+
 DISCUSSION
 ----------
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..07abfb8f83 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
 			git_config_get_bool("advice.amworkdir", &advice_amworkdir);
 
 			if (advice_amworkdir)
-				printf_ln(_("The copy of the patch that failed is found in: %s"),
-						am_path(state, "patch"));
+				printf_ln(_("Use 'git am --show-current-patch' to see the failed patch"));
 
 			die_user_resolve(state);
 		}
@@ -2121,6 +2120,23 @@ static void am_abort(struct am_state *state)
 	am_destroy(state);
 }
 
+static int show_patch(struct am_state *state)
+{
+	struct strbuf sb = STRBUF_INIT;
+	const char *patch_path;
+	int len;
+
+	patch_path = am_path(state, msgnum(state));
+	len = strbuf_read_file(&sb, patch_path, 0);
+	if (len < 0)
+		die_errno(_("failed to read '%s'"), patch_path);
+
+	setup_pager();
+	write_in_full(1, sb.buf, sb.len);
+	strbuf_release(&sb);
+	return 0;
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2165,8 @@ enum resume_mode {
 	RESUME_APPLY,
 	RESUME_RESOLVED,
 	RESUME_SKIP,
-	RESUME_ABORT
+	RESUME_ABORT,
+	RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2171,6 +2188,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	int patch_format = PATCH_FORMAT_UNKNOWN;
 	enum resume_mode resume = RESUME_FALSE;
 	int in_progress;
+	int ret = 0;
 
 	const char * const usage[] = {
 		N_("git am [<options>] [(<mbox> | <Maildir>)...]"),
@@ -2249,6 +2267,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_CMDMODE(0, "abort", &resume,
 			N_("restore the original branch and abort the patching operation."),
 			RESUME_ABORT),
+		OPT_CMDMODE(0, "show-current-patch", &resume,
+			N_("show the patch being applied."),
+			RESUME_SHOW_PATCH),
 		OPT_BOOL(0, "committer-date-is-author-date",
 			&state.committer_date_is_author_date,
 			N_("lie about committer date")),
@@ -2359,11 +2380,14 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	case RESUME_ABORT:
 		am_abort(&state);
 		break;
+	case RESUME_SHOW_PATCH:
+		ret = show_patch(&state);
+		break;
 	default:
 		die("BUG: invalid resume value");
 	}
 
 	am_state_release(&state);
 
-	return 0;
+	return ret;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3683c772c5..56ca445fa8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "--skip --continue --resolved --abort --show-current-patch"
 		return
 	fi
 	case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..23abf42abc 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am pauses on conflict' '
 	test -d .git/rebase-apply
 '
 
+test_expect_success 'am --show-current-patch' '
+	git am --show-current-patch >actual.patch &&
+	test_cmp .git/rebase-apply/0001 actual.patch
+'
+
 test_expect_success 'am --skip works' '
 	echo goodbye >expected &&
 	git am --skip &&
-- 
2.16.1.399.g632f88eed1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v3 2/3] rebase: add --show-current-patch
  2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
  2018-02-11  9:43     ` [PATCH v3 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
@ 2018-02-11  9:43     ` Nguyễn Thái Ngọc Duy
  2018-02-11  9:43     ` [PATCH v3 3/3] rebase: introduce and use pseudo-ref REBASE_HEAD Nguyễn Thái Ngọc Duy
  2018-02-22  0:21     ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Junio C Hamano
  3 siblings, 0 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-02-11  9:43 UTC (permalink / raw)
  To: git
  Cc: phillip.wood, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim, Nguyễn Thái Ngọc Duy

It is useful to see the full patch while resolving conflicts in a
rebase. The only way to do it now is

    less .git/rebase-*/patch

which could turn out to be a lot longer to type if you are in a
linked worktree, or not at top-dir. On top of that, an ordinary user
should not need to peek into .git directory. The new option is
provided to examine the patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           |  6 ++++-
 builtin/am.c                           | 11 +++++++++
 contrib/completion/git-completion.bash |  4 ++--
 git-rebase--am.sh                      |  3 +++
 git-rebase--interactive.sh             |  3 +++
 git-rebase--merge.sh                   |  3 +++
 git-rebase.sh                          |  7 +++++-
 t/t3400-rebase.sh                      | 33 ++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh          |  5 ++++
 9 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..7ef9577472 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[<upstream> [<branch>]]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
 	--root [<branch>]
-'git rebase' --continue | --skip | --abort | --quit | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo | --show-current-patch
 
 DESCRIPTION
 -----------
@@ -250,6 +250,10 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 --edit-todo::
 	Edit the todo list during an interactive rebase.
 
+--show-current-patch::
+	Show the current patch in an interactive rebase or when rebase
+	is stopped because of conflicts.
+
 -m::
 --merge::
 	Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/builtin/am.c b/builtin/am.c
index 07abfb8f83..37219fceb0 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2126,6 +2126,17 @@ static int show_patch(struct am_state *state)
 	const char *patch_path;
 	int len;
 
+	if (!is_null_oid(&state->orig_commit)) {
+		const char *av[4] = { "show", NULL, "--", NULL };
+		char *new_oid_str;
+		int ret;
+
+		av[1] = new_oid_str = xstrdup(oid_to_hex(&state->orig_commit));
+		ret = run_command_v_opt(av, RUN_GIT_CMD);
+		free(new_oid_str);
+		return ret;
+	}
+
 	patch_path = am_path(state, msgnum(state));
 	len = strbuf_read_file(&sb, patch_path, 0);
 	if (len < 0)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 56ca445fa8..2bd30d68cf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,11 +1992,11 @@ _git_rebase ()
 {
 	__git_find_repo_path
 	if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
-		__gitcomp "--continue --skip --abort --quit --edit-todo"
+		__gitcomp "--continue --skip --abort --quit --edit-todo --show-current-patch"
 		return
 	elif [ -d "$__git_repo_path"/rebase-apply ] || \
 	     [ -d "$__git_repo_path"/rebase-merge ]; then
-		__gitcomp "--continue --skip --abort --quit"
+		__gitcomp "--continue --skip --abort --quit --show-current-patch"
 		return
 	fi
 	__git_complete_strategy && return
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..c931891cbc 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
 	move_to_original_branch
 	return
 	;;
+show-current-patch)
+	exec git am --show-current-patch
+	;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..0c0f8abbf9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -840,6 +840,9 @@ To continue rebase after editing, run:
 
 	exit
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/stopped-sha")" --
+	;;
 esac
 
 comment_for_reflog start
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..0a96dfae37 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,9 @@ skip)
 	finish_rb_merge
 	return
 	;;
+show-current-patch)
+	exec git show "$(cat "$state_dir/current")" --
+	;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..41c915d18c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
+show-current-patch! show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--quit|--edit-todo)
+	--continue|--skip|--abort|--quit|--edit-todo|--show-current-patch)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -412,6 +413,10 @@ quit)
 edit-todo)
 	run_specific_rebase
 	;;
+show-current-patch)
+	run_specific_rebase
+	die "BUG: run_specific_rebase is not supposed to return here"
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 8ac58d5ea5..09943d6a9b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -277,4 +277,37 @@ EOF
 	test_cmp From_.msg out
 '
 
+test_expect_success 'rebase--am.sh and --show-current-patch' '
+	test_create_repo conflict-apply &&
+	(
+		cd conflict-apply &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --onto init HEAD^ &&
+		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+		grep "show.*$(git rev-parse two)" stderr
+	)
+'
+
+test_expect_success 'rebase--merge.sh and --show-current-patch' '
+	test_create_repo conflict-merge &&
+	(
+		cd conflict-merge &&
+		test_commit init &&
+		echo one >>init.t &&
+		git commit -a -m one &&
+		echo two >>init.t &&
+		git commit -a -m two &&
+		git tag two &&
+		test_must_fail git rebase --merge --onto init HEAD^ &&
+		git rebase --show-current-patch >actual.patch &&
+		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+		grep "show.*$(git rev-parse two)" stderr
+	)
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 481a350090..3af6f149a9 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -225,6 +225,11 @@ test_expect_success 'stop on conflicting pick' '
 	test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
 '
 
+test_expect_success 'show conflicted patch' '
+	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
+	grep "show.*$(cat "$state_dir/stopped-sha")" stderr
+'
+
 test_expect_success 'abort' '
 	git rebase --abort &&
 	test $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&
-- 
2.16.1.399.g632f88eed1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v3 3/3] rebase: introduce and use pseudo-ref REBASE_HEAD
  2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
  2018-02-11  9:43     ` [PATCH v3 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
  2018-02-11  9:43     ` [PATCH v3 2/3] rebase: " Nguyễn Thái Ngọc Duy
@ 2018-02-11  9:43     ` Nguyễn Thái Ngọc Duy
  2018-02-22  0:21     ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Junio C Hamano
  3 siblings, 0 replies; 40+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-02-11  9:43 UTC (permalink / raw)
  To: git
  Cc: phillip.wood, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine, tim, Nguyễn Thái Ngọc Duy

The new command `git rebase --show-current-patch` is useful for seeing
the commit related to the current rebase state. Some however may find
the "git show" command behind it too limiting. You may want to
increase context lines, do a diff that ignores whitespaces...

For these advanced use cases, the user can execute any command they
want with the new pseudo ref REBASE_HEAD.

This also helps show where the stopped commit is from, which is hard
to see from the previous patch which implements --show-current-patch.

Helped-by: Tim Landscheidt <tim@tim-landscheidt.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt           | 3 ++-
 builtin/am.c                           | 4 ++++
 contrib/completion/git-completion.bash | 2 +-
 git-rebase--interactive.sh             | 5 ++++-
 git-rebase--merge.sh                   | 4 +++-
 git-rebase.sh                          | 1 +
 sequencer.c                            | 4 ++++
 t/t3400-rebase.sh                      | 3 ++-
 t/t3404-rebase-interactive.sh          | 5 ++++-
 9 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7ef9577472..0b29e48221 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 
 --show-current-patch::
 	Show the current patch in an interactive rebase or when rebase
-	is stopped because of conflicts.
+	is stopped because of conflicts. This is the equivalent of
+	`git show REBASE_HEAD`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index 37219fceb0..21aedec41f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
 
 	if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
 		die_errno(_("failed to create directory '%s'"), state->dir);
+	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
 	if (split_mail(state, patch_format, paths, keep_cr) < 0) {
 		am_destroy(state);
@@ -1110,6 +1111,7 @@ static void am_next(struct am_state *state)
 
 	oidclr(&state->orig_commit);
 	unlink(am_path(state, "original-commit"));
+	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
 	if (!get_oid("HEAD", &head))
 		write_state_text(state, "abort-safety", oid_to_hex(&head));
@@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, const char *mail)
 
 	oidcpy(&state->orig_commit, &commit_oid);
 	write_state_text(state, "original-commit", oid_to_hex(&commit_oid));
+	update_ref("am", "REBASE_HEAD", &commit_oid,
+		   NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 
 	return 0;
 }
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2bd30d68cf..8777805c9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -439,7 +439,7 @@ __git_refs ()
 			track=""
 			;;
 		*)
-			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
+			for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD REBASE_HEAD; do
 				case "$i" in
 				$match*)
 					if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c0f8abbf9..a613156bcb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,12 +199,14 @@ make_patch () {
 
 die_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref REBASE_HEAD "$1"
 	make_patch "$1"
 	die "$2"
 }
 
 exit_with_patch () {
 	echo "$1" > "$state_dir"/stopped-sha
+	git update-ref REBASE_HEAD "$1"
 	make_patch $1
 	git rev-parse --verify HEAD > "$amend"
 	gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
@@ -841,7 +843,7 @@ To continue rebase after editing, run:
 	exit
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/stopped-sha")" --
+	exec git show REBASE_HEAD --
 	;;
 esac
 
@@ -858,6 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary \$state_dir")"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 0a96dfae37..957688f236 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -57,6 +57,7 @@ call_merge () {
 	echo "$msgnum" >"$state_dir/msgnum"
 	cmt="$(cat "$state_dir/cmt.$msgnum")"
 	echo "$cmt" > "$state_dir/current"
+	git update-ref REBASE_HEAD "$cmt"
 	hd=$(git rev-parse --verify HEAD)
 	cmt_name=$(git symbolic-ref HEAD 2> /dev/null || echo HEAD)
 	eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
@@ -138,13 +139,14 @@ skip)
 	return
 	;;
 show-current-patch)
-	exec git show "$(cat "$state_dir/current")" --
+	exec git show REBASE_HEAD --
 	;;
 esac
 
 mkdir -p "$state_dir"
 echo "$onto_name" > "$state_dir/onto_name"
 write_basic_state
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 msgnum=0
 for cmt in $(git rev-list --reverse --no-merges "$revisions")
diff --git a/git-rebase.sh b/git-rebase.sh
index 41c915d18c..a13a581fe6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -182,6 +182,7 @@ You can run "git stash pop" or "git stash drop" at any time.
 }
 
 finish_rebase () {
+	rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 	apply_autostash &&
 	{ git gc --auto || true; } &&
 	rm -rf "$state_dir"
diff --git a/sequencer.c b/sequencer.c
index 4d3f60594c..f692221999 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1792,6 +1792,9 @@ static int make_patch(struct commit *commit, struct replay_opts *opts)
 	p = short_commit_name(commit);
 	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
 		return -1;
+	if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
+		       NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
+		res |= error(_("could not update %s"), "REBASE_HEAD");
 
 	strbuf_addf(&buf, "%s/patch", get_dir(opts));
 	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@ -2043,6 +2046,7 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			unlink(rebase_path_author_script());
 			unlink(rebase_path_stopped_sha());
 			unlink(rebase_path_amend());
+			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 		}
 		if (item->command <= TODO_SQUASH) {
 			if (is_rebase_i(opts))
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 09943d6a9b..72d9564747 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -306,7 +306,8 @@ test_expect_success 'rebase--merge.sh and --show-current-patch' '
 		test_must_fail git rebase --merge --onto init HEAD^ &&
 		git rebase --show-current-patch >actual.patch &&
 		GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-		grep "show.*$(git rev-parse two)" stderr
+		grep "show.*REBASE_HEAD" stderr &&
+		test "$(git rev-parse REBASE_HEAD)" = "$(git rev-parse two)"
 	)
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3af6f149a9..23a54a4c49 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -227,7 +227,10 @@ test_expect_success 'stop on conflicting pick' '
 
 test_expect_success 'show conflicted patch' '
 	GIT_TRACE=1 git rebase --show-current-patch >/dev/null 2>stderr &&
-	grep "show.*$(cat "$state_dir/stopped-sha")" stderr
+	grep "show.*REBASE_HEAD" stderr &&
+	# the original stopped-sha1 is abbreviated
+	stopped_sha1="$(git rev-parse $(cat ".git/rebase-merge/stopped-sha"))" &&
+	test "$(git rev-parse REBASE_HEAD)" = "$stopped_sha1"
 '
 
 test_expect_success 'abort' '
-- 
2.16.1.399.g632f88eed1


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"
  2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
                       ` (2 preceding siblings ...)
  2018-02-11  9:43     ` [PATCH v3 3/3] rebase: introduce and use pseudo-ref REBASE_HEAD Nguyễn Thái Ngọc Duy
@ 2018-02-22  0:21     ` Junio C Hamano
  2018-02-22  0:54       ` Tim Landscheidt
  3 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2018-02-22  0:21 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, phillip.wood, Ævar Arnfjörð Bjarmason,
	Johannes Schindelin, Eric Sunshine, tim

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Compared to v2:
>
> - the potential loss of errno before it's printed out in builtin/am.c
>   is fixed.
> - keep update_ref() in sequencer.c non-fatal like this rest
> - rename ORIG_COMMIT to REBASE_HEAD
>
> Interdiff:

This round hasn't seen any comments.  Is everybody happy with it?

I personally do not have strong opinion for the feature but didn't
spot anything against the execution, either, so...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"
  2018-02-22  0:21     ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Junio C Hamano
@ 2018-02-22  0:54       ` Tim Landscheidt
  2018-02-23 10:20         ` Duy Nguyen
  0 siblings, 1 reply; 40+ messages in thread
From: Tim Landscheidt @ 2018-02-22  0:54 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc Duy, git, phillip.wood,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine

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

>> Compared to v2:

>> - the potential loss of errno before it's printed out in builtin/am.c
>>   is fixed.
>> - keep update_ref() in sequencer.c non-fatal like this rest
>> - rename ORIG_COMMIT to REBASE_HEAD

>> Interdiff:

> This round hasn't seen any comments.  Is everybody happy with it?

> I personally do not have strong opinion for the feature but didn't
> spot anything against the execution, either, so...

Sorry for the late reply: I dislike REBASE_/HEAD/ because
ORIG_/HEAD/ refers to the tip of the original branch, and
/ORIG/_HEAD refers to the original branch, so
/REBASE/_/HEAD/ is doubly confusing IMHO.  I consider
ORIG_COMMIT easier to understand because ORIG_HEAD refers to
the tip of the original branch, and ORIG_COMMIT would refer
to one of the commits making up that original branch, but as
I suggested it myself I might not be very objective in that
regard :-).

(BTW, thanks, Duy, for doing the actual work!)

Tim

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"
  2018-02-22  0:54       ` Tim Landscheidt
@ 2018-02-23 10:20         ` Duy Nguyen
  0 siblings, 0 replies; 40+ messages in thread
From: Duy Nguyen @ 2018-02-23 10:20 UTC (permalink / raw)
  To: Tim Landscheidt
  Cc: Junio C Hamano, Git Mailing List, Phillip Wood,
	Ævar Arnfjörð Bjarmason, Johannes Schindelin,
	Eric Sunshine

On Thu, Feb 22, 2018 at 7:54 AM, Tim Landscheidt <tim@tim-landscheidt.de> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>
>>> Compared to v2:
>
>>> - the potential loss of errno before it's printed out in builtin/am.c
>>>   is fixed.
>>> - keep update_ref() in sequencer.c non-fatal like this rest
>>> - rename ORIG_COMMIT to REBASE_HEAD
>
>>> Interdiff:
>
>> This round hasn't seen any comments.  Is everybody happy with it?
>
>> I personally do not have strong opinion for the feature but didn't
>> spot anything against the execution, either, so...
>
> Sorry for the late reply: I dislike REBASE_/HEAD/ because
> ORIG_/HEAD/ refers to the tip of the original branch, and
> /ORIG/_HEAD refers to the original branch, so
> /REBASE/_/HEAD/ is doubly confusing IMHO.  I consider
> ORIG_COMMIT easier to understand because ORIG_HEAD refers to
> the tip of the original branch, and ORIG_COMMIT would refer
> to one of the commits making up that original branch, but as
> I suggested it myself I might not be very objective in that
> regard :-).

I wonder if you could make a ref symlink named ORIG_COMMIT pointing to
REBASE_HEAD. A bit more setup for you, but that'll make everybody
happy.
-- 
Duy

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2018-02-23 10:21 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-26  9:55 [PATCH 0/2] Add "git rebase --show-patch" Nguyễn Thái Ngọc Duy
2018-01-26  9:55 ` [PATCH 1/2] am: add --show-patch Nguyễn Thái Ngọc Duy
2018-01-26  9:55 ` [PATCH 2/2] rebase: " Nguyễn Thái Ngọc Duy
2018-01-26 11:12   ` Phillip Wood
2018-01-26 11:22     ` Duy Nguyen
2018-01-30 11:15       ` Phillip Wood
2018-01-26 19:11   ` Eric Sunshine
2018-01-27  1:42     ` Duy Nguyen
2018-01-26 18:47 ` [PATCH 0/2] Add "git rebase --show-patch" Tim Landscheidt
2018-01-27  1:45   ` Duy Nguyen
2018-01-27 11:26 ` Ævar Arnfjörð Bjarmason
2018-01-29 15:09 ` Johannes Schindelin
2018-01-30  9:10   ` Duy Nguyen
2018-01-30 12:32     ` Johannes Schindelin
2018-01-30 20:19       ` Junio C Hamano
2018-02-01  2:06         ` Tim Landscheidt
2018-01-31  9:30 ` [PATCH v2 0/3] " Nguyễn Thái Ngọc Duy
2018-01-31  9:30   ` [PATCH v2 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
2018-01-31  9:40     ` Eric Sunshine
2018-01-31 22:59       ` Junio C Hamano
2018-02-02  9:25         ` Duy Nguyen
2018-02-02  9:46           ` Eric Sunshine
2018-02-02  9:53             ` Duy Nguyen
2018-02-02 18:56           ` Junio C Hamano
2018-01-31  9:30   ` [PATCH] gitignore.txt: elaborate shell glob syntax Nguyễn Thái Ngọc Duy
2018-01-31 23:22     ` Junio C Hamano
2018-02-01  9:59       ` Duy Nguyen
2018-01-31  9:30   ` [PATCH v2 2/3] rebase: add --show-current-patch Nguyễn Thái Ngọc Duy
2018-01-31  9:30   ` [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT Nguyễn Thái Ngọc Duy
2018-01-31 23:18     ` Junio C Hamano
2018-02-01 10:02       ` Duy Nguyen
2018-02-02 19:00         ` Junio C Hamano
2018-02-01 11:05     ` Phillip Wood
2018-02-11  9:43   ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Nguyễn Thái Ngọc Duy
2018-02-11  9:43     ` [PATCH v3 1/3] am: add --show-current-patch Nguyễn Thái Ngọc Duy
2018-02-11  9:43     ` [PATCH v3 2/3] rebase: " Nguyễn Thái Ngọc Duy
2018-02-11  9:43     ` [PATCH v3 3/3] rebase: introduce and use pseudo-ref REBASE_HEAD Nguyễn Thái Ngọc Duy
2018-02-22  0:21     ` [PATCH v3 0/3] Add "git rebase --show-current-patch" Junio C Hamano
2018-02-22  0:54       ` Tim Landscheidt
2018-02-23 10:20         ` Duy Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).