Ramkumar Ramachandra [off-list ref] writes:
Junio C Hamano wrote:
quoted
As I said (twice), you can argue that that particular piece of
information is not useful (at least to you), but why it is not
useful has to be justified, against the justification given by
b397ea4863a1 (status: show more info than "currently not on any
branch", 2013-03-13) and people who have been using that information
in the status output, no?
Yes, Junio. I cannot justify that "detached HEAD from" is completely
useless, because it is not. I argued for a practical compromise, and
have tried to show the huge negative impact against the marginal gain.
First of all, during a stopped "git rebase", I think "git status"
that says "# HEAD detached at" or "# HEAD detached from" is not
something we want in the end result anyway. In either case, that
comes from the internal use of "git checkout" by the command to
switch out of a concrete branch, which is *not* user initiated.
If you first update "git checkout" so that it will pay attention to
a custom reflog-action exported by Porcelain scripts that may want
to internally use it to flip branches (and without a custom one, it
will still record "checkout: moving from A to B"), without exporting
custom reflog-action from "rebase" and other Porcelain scripts, that
would not affect any externally visible behaviour.
When you teach "git rebase" to pass custom reflog-action when it
detaches the HEAD at "$onto^0" (because that is not a user-initiated
action) as the next step, nth_prior_checkout and 1st_switch would
stop counting them as "checkout" events. That fix will give you the
desired result of the topic of this series, i.e. "git checkout @{-1}"
ignores the internal "git checkout" done by "rebase".
That step may change the output from "git status" but I do not think
the output from "git status" in this sequence:
git rebase ;# stops upon conflict
git status
is anything sacred. Tests modified by b397ea4863a1 (status: show
more info than "currently not on any branch", 2013-03-13) do change
the original "# Not currently on any branch" to "detached at", but I
do not think b397ea4863a1 _wanted_ to say where the $onto commit was
during the rebase (or was it?), so if they have to change to
"detached from", I do not think it is a problem at all. I think it
is very sane to update these selected "detached at" to "detached
from" as needed.
That would be a sensible compromise, because we do not want to see
either "detached at" or "detached from" during "rebase" in the
longer term. We would rather want to see the message start with
something like
# Rebasing X on top of Y (23 commits remaining to be replayed)
which is under discussion in a separate thread lately. The change
to expected output (i.e. "detached at" to "detached from") will have
to be changed again in that separate topic.
The very last test that the b397ea4863a1 added, which wants to see
"detached at atag", should be kept, and also there should be another
test added to reset one step back to make sure we see "detached
from" after it. It tests what the user sees after a "git checkout"
initiated by the end-user.
Junio C Hamano wrote:
If you first update "git checkout" so that it will pay attention to
a custom reflog-action exported by Porcelain scripts that may want
to internally use it to flip branches (and without a custom one, it
will still record "checkout: moving from A to B"), without exporting
custom reflog-action from "rebase" and other Porcelain scripts, that
would not affect any externally visible behaviour.
rebase already sets a custom reflog-action; in fact,
'set_reflog_action rebase' is executed after it sources sh-setup. I
have to go out of my way to deliberately unset it before every
checkout invocation to avoid breaking all the tests in t/status-help.
Most unnatural way to go about writing a series, if you ask me.
Tests modified by b397ea4863a1 (status: show
more info than "currently not on any branch", 2013-03-13) do change
the original "# Not currently on any branch" to "detached at", but I
do not think b397ea4863a1 _wanted_ to say where the $onto commit was
during the rebase (or was it?), so if they have to change to
"detached from", I do not think it is a problem at all. I think it
is very sane to update these selected "detached at" to "detached
from" as needed.
... and you have missed the point yet again.
I previously wrote:
I'd just like to point out quickly that I first attempted to write 6/6
without this patch. It is absolutely impossible, because the
"detached HEAD from/to" messages no longer make any sense when
checkout starts respecting GIT_REFLOG_ACTION. At that point, I'm was
just monkeying around the trash-directory running describes to somehow
try and make the expected output equal to the actual output. There
was no method to the madness, and I was literally losing my mind.
Let me try and explain this once again: to determine "HEAD detached
from/to", wt-status must look for a previous "checkout: " message in
the reflog. Since a rebase no longer executes a checkout, the
"detached HEAD from" message no longer has anything to do with the
rebase itself; it is completely _random_ and _incidental_, based on
what checkout operation was executed last (eg. in a previous test).
To change the "detached HEAD to" messages to "detached HEAD from"
messages, I have to turn myself into a monkey that goes around running
describes on the test-directory. After enduring hours of this
mind-numbing job, the final result will be a terrible mess; tests can
no longer be added, removed, reordered, or even tweaked. Everything
depends on the exact ordering of the tests, and the incidental
checkout invocations they contain.
That would be a sensible compromise
How is it a compromise? It is most uncompromising.
because we do not want to see
either "detached at" or "detached from" during "rebase" in the
longer term. We would rather want to see the message start with
something like
# Rebasing X on top of Y (23 commits remaining to be replayed)
which is under discussion in a separate thread lately. The change
to expected output (i.e. "detached at" to "detached from") will have
to be changed again in that separate topic.
Orthogonal. I want checkout-dash now, and I should not have to be
asked to do something as unrelated as redoing 'git status' output to
get what I want.
Since you have made it clear that you will defend every bit of b397ea
against all odds, and since I want checkout-dash now, we have to do
something. I have a proposal: I will smudge the first line of the
'git status' output, before comparing it to make tests pass. Never
mind that the line is incidental and Wrong; since I have no say in the
matter, I will just sidestep the issue. I don't use the long-form
'git status' anyway, and am never going to see this ugliness. We both
win. Okay?