Junio C Hamano [off-list ref] writes:
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.
And it might be just the matter of doing something like this
(totally untested, of course). Then the people who want to improve
"status" to give more detailed state during "rebase" can further
enhance the logic in this part to give more detailed information.
I have a feeling that without changing anything in t7512 but
applying the change to commit.c in your [6/6], most tests in there
would still pass, and you may even uncover latent *bug* that
expected to show a wrong $ONTO value in "# HEAD detached from/at"
line.
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;
if (!get_sha1("HEAD", sha1) &&
Junio C Hamano wrote:
quoted 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.