On Mon, Jul 16, 2012 at 11:59:45AM +0200, Matthieu Moy wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > Subject: [PATCH] status: color in-progress message like other header messages
>
> My feeling is that these "in progress" messages would deserve to be more
> visible than the usual headers (like "Not currently on any branch.",
> which is both legit and likely to be a user-error). For example, the
> other day, the patch told me I was bisecting. I thought it was a bug in
> the patch, but it was indeed a user-error of mine, I did not end this
> bisect session properly around a month ago ;-).
>
> This would argue in favor of having a different, configurable color.
Sure, I have no problem with that reasoning. Only the half-done
implementation. :)
> But as the code currently does not allow user-configuration anyway, it's
> probably best to make the code clean and uniform. If someone wants to
> add customizability afterwards, it won't be that hard (it's probably a
> good idea to have people leave with these messages for a while before
> deciding what color it should be).
>
> This second patch looks better to me, but no strong opinion here.
Let's go with the second patch for now, then. It fixes the immediate
problem, and it does not make it much harder for somebody to do a
separate configurable color on top if they want to do so (now or later).
They would have to re-add the slot, but that is the least of the effort
involved; handling the partial-line coloring and the default-to-header
as ONBRANCH does is the more interesting bit, and the current code does
not do that at all.
Here's a repost with a slightly modified commit message (on top of
lk/more-helpful-status-hints, as before).
-- >8 --
Subject: status: color in-progress message like other header messages
The "status" command recently learned to describe the
in-progress operation in its long output format (e.g.,
rebasing, am, etc). This message gets its own slot in the
color table, even though it is not configurable. As a
result, if the user has set color.status.header to a
non-default value, this message will not match (and cannot
be made to match, as there is no config option).
It is probably more sane to just color it like the rest of
the text (i.e., just use color.status.header). This would
not allow users to customize the color of this message
independently, but they cannot do that with the current code
anyway, and if somebody wants to build customizable
colorization later, this patch does not make it much harder
to do so.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
wt-status.c | 3 +--
wt-status.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index c749267..c110cbc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -24,7 +24,6 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
GIT_COLOR_GREEN, /* WT_STATUS_LOCAL_BRANCH */
GIT_COLOR_RED, /* WT_STATUS_REMOTE_BRANCH */
GIT_COLOR_NIL, /* WT_STATUS_ONBRANCH */
- GIT_COLOR_NORMAL, /* WT_STATUS_IN_PROGRESS */
};
static const char *color(int slot, struct wt_status *s)
@@ -931,7 +930,7 @@ static void show_bisect_in_progress(struct wt_status *s,
static void wt_status_print_state(struct wt_status *s)
{
- const char *state_color = color(WT_STATUS_IN_PROGRESS, s);
+ const char *state_color = color(WT_STATUS_HEADER, s);
struct wt_status_state state;
struct stat st;
diff --git a/wt-status.h b/wt-status.h
index c1066a0..f8fc58c 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -15,7 +15,6 @@ enum color_wt_status {
WT_STATUS_LOCAL_BRANCH,
WT_STATUS_REMOTE_BRANCH,
WT_STATUS_ONBRANCH,
- WT_STATUS_IN_PROGRESS,
WT_STATUS_MAXSLOT
};
--
1.7.10.5.40.gbbc17de
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html