Re: [PATCH 5/6] status: do not depend on flaky reflog messages

Ramkumar Ramachandra [off-list ref] writes:

Junio C Hamano wrote:

quoted

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.

...
The question is: what do _you_ want now?

It should be fairly clear what _I_ want now.  Re-read the first four
lines you quoted above.

Read the various "HEAD detached from/to" messages in the rebase tests
in t/status-help and tell me that they make some sense.

Again, re-read the part you did _not_ quote before those four lines.
I do not think all of them make lot of sense; some of them may.

Unless you are also tackling the topic of the other thread:

  http://thread.gmane.org/gmane.comp.version-control.git/227432/focus=227469

(which I would not suggest to do in this topic), what these lines
say during "rebase" does not really matter, as far as your topic is
concerned.

Two possibilities:

 (1) Assume that the other thread will produce a more reasonable
     semantics when finished; perhaps the first line will go away
     entirely, or maybe it would say something like "# Rebasing;
     head at $commit".

     Your topic does not _care_ what it would say, so you tweak the
     "status" test that is done during "rebase" so that they
     ignore the first lines; or

 (2) Starting from the same assumption as above, but try to minimize
     the semantics change to user-visible behaviour this series
     makes.

     That means that even though the _primary_ thing you want to do
     is to tweak "rebase" and its internal use of "checkout" in such
     a way that reflog will not record the implementation-detail
     checkout (because that will affect the next "checkout -"), make
     sure that "status" while doing "rebase" reports where the
     internal "checkout" of $ONTO detached HEAD from/at.

The "something along this line" patch was an illustration of how
the beginning of the effort for (2) would look like.

As I already said, I could see us going either way.  If we can do
(2), that may be better, but at end it would not matter that much as
long as we will be doing the other topic, too.

Junio C Hamano wrote:

Two possibilities:

 (1) Assume that the other thread will produce a more reasonable
     semantics when finished; perhaps the first line will go away
     entirely, or maybe it would say something like "# Rebasing;
     head at $commit".

     Your topic does not _care_ what it would say, so you tweak the
     "status" test that is done during "rebase" so that they
     ignore the first lines; or

You said you didn't want to regress to show senseless information, and
I agreed with that.  What is wrong with the patch I showed in the
previous email?  Smudging is a bad hack, and must only be used as a
last resort: when an another topics updates status to say something
sensible, it will have to unsmudge the tests.

diff --git a/wt-status.c b/wt-status.c
index bf84a86..99c55e3 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -1182,7 +1182,7 @@ void wt_status_print(struct wt_status *s)
                                if (!get_sha1("HEAD", sha1) &&
                                    !hashcmp(sha1, state.detached_sha1))
                                        on_what = _("HEAD detached at ");
-                               else
+                               else if (!state.rebase_in_progress)
                                        on_what = _("HEAD detached from ");
                        } else {
                                branch_name = "";

 (2) Starting from the same assumption as above, but try to minimize
     the semantics change to user-visible behaviour this series
     makes.

The "try to minimize" is a somewhat admirable goal, but I have shown
that your midway solution is wrong.  Either dedicate a lot of time and
effort towards improving status for rebase, or don't attempt it.

     That means that even though the _primary_ thing you want to do
     is to tweak "rebase" and its internal use of "checkout" in such
     a way that reflog will not record the implementation-detail
     checkout (because that will affect the next "checkout -"), make
     sure that "status" while doing "rebase" reports where the
     internal "checkout" of $ONTO detached HEAD from/at.

Unless we change the first line drastically to say: "rebase in
progress: rebasing onto $ONTO" (or something), I don't think this
makes sense.  And if we were to do that, why not do it properly like
"rebase ($N/$M): onto $ONTO, upstream $UPSTREAM, branch $BRANCH"?
Other people on a different thread are already handling that, and I am
not interested.

So, you have three simple choices now:

1. Accept the simple patch I proposed above.
2. Propose an alternative patch quickly.  *Patch*.  No more English.
3. Reject all patches, and leave me no choice but to smudge.

Which one is it going to be?