Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"

[Duy Nguyen]

On Thu, Feb 22, 2018 at 7:54 AM, Tim Landscheidt  
wrote:
> Junio C Hamano  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

Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"

[Tim Landscheidt]

Junio C Hamano  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

Re: [PATCH v3 0/3] Add "git rebase --show-current-patch"

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   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...

[PATCH v3 0/3] Add "git rebase --show-current-patch"

[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);
@@ -,7 +,7 @@ static void am_next(struct am_state *state)
 
oidclr(>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", ))
write_state_text(state, "abort-safety", oid_to_hex());
@@ -1443,8 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
 
oidcpy(>orig_commit, _oid);
write_state_text(state, "original-commit", oid_to_hex(_oid));
-   update_ref("am", "ORIG_COMMIT", _oid,
-  NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+   update_ref("am", "REBASE_HEAD", _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(>orig_commit)) {
@@ -2140,10 +2141,10 @@ static int show_patch(struct am_state *state)
return ret;
}
 
-   len = strbuf_read_file(, am_path(state, msgnum(state)), 0);
+   patch_path = am_path(state, msgnum(state));
+   len = strbuf_read_file(, 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
+++