Ramkumar Ramachandra [off-list ref] writes:
Since you have made it clear that you will defend every bit of b397ea
You are misreading me. I am not defending every bit at all.
We both agree that what b397ea4863a1 (status: show more info than
"currently not on any branch", 2013-03-13) expects from "git status"
during an interrupted "git rebase" is _not_ sacred, especially when
we are about to fix what "git rebase" records in the reflog.
I am only saying that the last test the commit adds must be kept
unbroken. I am also saying that, even though that commit did not
add a test for "detached from" case, we should add something like
the attached to protect the behaviour. These two are sacred.
What happens to be shown during a stopped "git rebase" is not.
I have been assuming the "main" thing Duy wanted to do was the last
test (and the one below), but was this meant as an improvement for
"git status" output during that state? Showing $ONTO certainly
makes some sense, and from that point of view, the change we are
discussing _will_ be a regression if it just shows a random thing.
If you want to avoid regression, the codepath in wt-status.c should
compensate for the change to "rebase" so that it checks $dotest/onto
and show what is recorded in there.
But in practice, as far as we know other people are working on it in
the near-by thread, I do not think this particular regression (if it
was) matters that much. At the worst, we can queue that on top of
this topic to fix "rebase ; checkout -" and have them graduate at
the same time to avoid it.
t/t7512-status-help.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index bf08d4e..e07185e 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -675,6 +675,14 @@ test_expect_success 'status showing detached from a tag' '
nothing to commit (use -u to show untracked files)
EOF
git status --untracked-files=no >actual &&
+ test_i18ncmp expected actual &&
+
+ git reset --hard HEAD^ &&
+ cat >expected <<-\EOF
+ # HEAD detached from atag
+ nothing to commit (use -u to show untracked files)
+ EOF
+ git status --untracked-files=no >actual &&
test_i18ncmp expected actual
'
Junio C Hamano wrote:
I am only saying that the last test the commit adds must be kept
unbroken. I am also saying that, even though that commit did not
add a test for "detached from" case, we should add something like
the attached to protect the behaviour. These two are sacred.
What happens to be shown during a stopped "git rebase" is not.
I have been assuming the "main" thing Duy wanted to do was the last
test (and the one below), but was this meant as an improvement for
"git status" output during that state?
On what basis are you making that assumption? b397ea4 did not add a
test for "detached HEAD from", and there is no evidence of what the
author wanted in the commit message. What has happened has happened;
the question is: what do _you_ want now?
Read the various "HEAD detached from/to" messages in the rebase tests
in t/status-help and tell me that they make some sense. It does _not_
show a constant "HEAD detached from $onto", contrary to your
assumption (evidenced by your patch in the later email). Look at these
hunk added by b397ea4, and tell me that it's not a monkey-patch:
@@ -187,9 +187,10 @@ test_expect_success 'status when rebasing -i in
edit mode' '
export FAKE_LINES &&
test_when_finished "git rebase --abort" &&
ONTO=$(git rev-parse --short HEAD~2) &&
+ TGT=$(git rev-parse --short two_rebase_i) &&
git rebase -i HEAD~2 &&
cat >expected <<-EOF &&
- # Not currently on any branch.
+ # HEAD detached from $TGT
# You are currently editing a commit while rebasing branch
'\''rebase_i_edit'\'' on '\''$ONTO'\''.
# (use "git commit --amend" to amend the current commit)
# (use "git rebase --continue" once you are satisfied with
your changes)@@ -214,8 +215,9 @@ test_expect_success 'status when splitting a commit' '
ONTO=$(git rev-parse --short HEAD~3) &&
git rebase -i HEAD~3 &&
git reset HEAD^ &&
+ TGT=$(git rev-parse --short HEAD) &&
cat >expected <<-EOF &&
- # Not currently on any branch.
+ # HEAD detached at $TGT
# You are currently splitting a commit while rebasing branch'\''split_commit'\'' on '\''$ONTO'\''.
# (Once your working directory is clean, run "git rebase --continue")
#@@ -243,10 +245,11 @@ test_expect_success 'status after editing the
last commit with --amend during a
export FAKE_LINES &&
test_when_finished "git rebase --abort" &&
ONTO=$(git rev-parse --short HEAD~3) &&
+ TGT=$(git rev-parse --short three_amend) &&
git rebase -i HEAD~3 &&
git commit --amend -m "foo" &&
cat >expected <<-EOF &&
- # Not currently on any branch.
+ # HEAD detached from $TGT
# You are currently editing a commit while rebasing branch
'\''amend_last'\'' on '\''$ONTO'\''.
# (use "git commit --amend" to amend the current commit)
# (use "git rebase --continue" once you are satisfied with
your changes)
Showing $ONTO certainly
makes some sense, and from that point of view, the change we are
discussing _will_ be a regression if it just shows a random thing.
If showing $ONTO were the intent of the patch, this is no way to go
about it. Reliable information is readily available in
.git/rebase-apply; no parsing is required, and it is ready for
consumption.
Your argument now is: because the patch _incidentally_ shows $ONTO in
many cases (as evidenced by the hunks above), the objective of the
patch _must_ have been to show $ONTO. Should I laugh or cry?
If you want to avoid regression, the codepath in wt-status.c should
compensate for the change to "rebase" so that it checks $dotest/onto
and show what is recorded in there.
Now you want to achieve the exact same accidental behavior after
patching rebase/checkout. This is a never-ending nightmare.
You are misreading me. I am not defending every bit at all.
I am not misreading anything. Despite me having repeatedly shown that
b397ea4 is poorly done with far-reaching unintended consequences, you
are unwilling to admit it. Instead, you are trying to somehow
preserve this accidental behavior. And you still haven't been able to
show me how to do that. As a result, checkout-dash is stalled.