Re: [PATCH 5/6] status: do not depend on flaky reflog messages
From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:57:45
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH 5/6] status: do not depend on flaky reflog messages · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH 5/6] status: do not depend on flaky reflog messages · Ramkumar Ramachandra <hidden>
- 2016-06-15 · Re: [PATCH 5/6] status: do not depend on flaky reflog messages · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH 5/6] status: do not depend on flaky reflog messages · Ramkumar Ramachandra <hidden>
- 2016-06-15 · Re: [PATCH 5/6] status: do not depend on flaky reflog messages · Junio C Hamano <hidden>
Junio C Hamano wrote:
quoted hunk ↗ jump to hunk
wt-status.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)diff --git a/wt-status.c b/wt-status.c index bf84a86..403d48d 100644 --- a/wt-status.c +++ b/wt-status.c@@ -1176,7 +1176,11 @@ void wt_status_print(struct wt_status *s) branch_name += 11; else if (!strcmp(branch_name, "HEAD")) { branch_status_color = color(WT_STATUS_NOBRANCH, s); - if (state.detached_from) { + + if (state.rebase_in_progress) { + on_what = _("HEAD detached at "); + branch_name = state.onto; + } else if (state.detached_from) { unsigned char sha1[20]; branch_name = state.detached_from;
Good. You have proposed a solution to the problem. However, it is wrong for the following reasons: 1. It shows a pseudo "HEAD detached at" message. Everywhere else, when the user sees a "HEAD detached at $committish" message, git rev-parse $committish = git rev-parse HEAD. A rebase-in-progress seems to be the only exception, and the user has no idea this is happened. 2. The following no longer updates status: # in the middle of a rebase $ git reset @~2 The constant "HEAD detached at $onto" message is misleading and Bad. Besides, wasn't this the primary usecase you wanted? You previously wrote:
*1* One thing I could think of is to start sightseeing or (more
realistically) manually bisecting at a given release point,
reset the detached HEAD around several times, and then want to
be reminded where the session started from. I do not think it
is particularly a very good example, though.Whether the HEAD is detached by bisect or rebase is irrelevant. 3. The problem is not unique to rebase at all; yet you have special-cased it. If this isn't a band-aid, what is? The larger problem, as I have stated previously, is that 'git status' output depends on the _implementations_ of various commands (do they write a "checkout: " message to the reflog or not?). Therefore, a future contributor who updates a command to write more sensible reflog messages will have to apply a similar band-aid. If you want to take the band-aid approach, I think this is the right way to do it:
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 = "";
You have already mentioned that there is a topic cooking to improve this first-line in the case of rebase, so this regression from a senseless message isn't a problem. The problem with this bad-aid, as with all band aids, is that this will soon explode to become else if(!state.rebase_in_progress && !state.bisect_in_progress && ....) when people update those scripts. If you don't want to go with the band-aid, I have no choice but to smudge the first-line.