[PATCH] Updated status to show 'Not currently on any branch' in red

[PATCH] Updated status to show 'Not currently on any branch' in red

[Chris Parsons]

Hi all, first post and patch, please be gentle :)

I'm a fairly new user to git and have more than once been burnt by  
checking out an arbitrary commit and then checking in code on top of  
that commit. If you realise your mistake quickly, you can reset and  
reapply the commit to a branch, but if you checkout another branch  
your commit can disappear and become hard to find.

Therefore as a help to new users I've turned the 'Not currently on any  
branch' red on 'git status' so that it's harder to miss the fact.

Perhaps git should not allow commits in this case? I'm too much of a  
novice to know whether that's ever needed, but perhaps someone has a  
good reason for allowing it.

Thanks and regards
Chris

Signed-off-by: Chris Parsons <chris@edendevelopment.co.uk>
---
  wt-status.c |    7 +++++--
  wt-status.h |    1 +
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index a44c543..42a1ff8 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -18,6 +18,7 @@ static char wt_status_colors[][COLOR_MAXLEN] = {
  	"\033[32m", /* WT_STATUS_UPDATED: green */
  	"\033[31m", /* WT_STATUS_CHANGED: red */
  	"\033[31m", /* WT_STATUS_UNTRACKED: red */
+	"\033[31m", /* WT_STATUS_NOBRANCH: red */
  };

  static const char use_add_msg[] =
@@ -315,7 +316,8 @@ void wt_status_print(struct wt_status *s)
  {
  	unsigned char sha1[20];
  	s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
-
+  const char* branch_color = color(WT_STATUS_HEADER);
+
  	if (s->branch) {
  		const char *on_what = "On branch ";
  		const char *branch_name = s->branch;
@@ -323,9 +325,10 @@ void wt_status_print(struct wt_status *s)
  			branch_name += 11;
  		else if (!strcmp(branch_name, "HEAD")) {
  			branch_name = "";
+			branch_color = color(WT_STATUS_NOBRANCH);
  			on_what = "Not currently on any branch.";
  		}
-		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER),
+		color_fprintf_ln(s->fp, branch_color,
  			"# %s%s", on_what, branch_name);
  	}

diff --git a/wt-status.h b/wt-status.h
index 7d61410..f0675fd 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -8,6 +8,7 @@ enum color_wt_status {
  	WT_STATUS_UPDATED,
  	WT_STATUS_CHANGED,
  	WT_STATUS_UNTRACKED,
+	WT_STATUS_NOBRANCH,
  };

  struct wt_status {
-- 
1.5.5.1.249.g3fdb.dirty

Re: [PATCH] Updated status to show 'Not currently on any branch' in red

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Thu, May 15, 2008 at 08:37:52PM +0100, Chris Parsons <chris@edendevelopment.co.uk> wrote:
> Hi all, first post and patch, please be gentle :)

In general, please write such comments after the --- and before the
diffstat, so it won't be part of the commit message. More tricks in
Documentation/SubmittingPatches. :)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Updated status to show 'Not currently on any branch' in red

[Jeff King]

On Thu, May 15, 2008 at 08:37:52PM +0100, Chris Parsons wrote:

> Hi all, first post and patch, please be gentle :)

Hi, and welcome.

But as Miklos pointed out, please separate "cover letter" material from
the commit message by putting it under the "---" cut.

> I'm a fairly new user to git and have more than once been burnt by  
> checking out an arbitrary commit and then checking in code on top of that 
> commit. If you realise your mistake quickly, you can reset and reapply the 
> commit to a branch, but if you checkout another branch your commit can 
> disappear and become hard to find.
>
> Therefore as a help to new users I've turned the 'Not currently on any  
> branch' red on 'git status' so that it's harder to miss the fact.

I think this is definitely a good change overall, but I have some
specific comments on the implementation below.

> Perhaps git should not allow commits in this case? I'm too much of a  
> novice to know whether that's ever needed, but perhaps someone has a good 
> reason for allowing it.

There was much discussion of this when the detached HEAD feature was
introduced, but it was decided that allowing commits was useful. I think
putting the text in red is a nice way to highlight a potential mistake,
though.

>  static const char use_add_msg[] =
> @@ -315,7 +316,8 @@ void wt_status_print(struct wt_status *s)
>  {
>  	unsigned char sha1[20];
>  	s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
> -
> +  const char* branch_color = color(WT_STATUS_HEADER);
> +

Indentation?

> -		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER),
> +		color_fprintf_ln(s->fp, branch_color,
>  			"# %s%s", on_what, branch_name);

This makes the whole line red; I think it would look nicer matching the
red / green files last on in the file list by coloring just the message
part. IOW,

  color_fprintf(s->fp, color(WT_STATUS_HEADER), "# ");
  color_fprintf_ln(s->fp, branch_color, %s%s", on_what, branch_name);


And two final points on missing items:

  1. This should probably be configurable. You'd need to update
     parse_status_slot, as well as the documentation in config.txt.

  2. Note that this just covers the colorization of status sent to the
     terminal or pager. What is seen in the editor while writing the
     commit message (which is a likely place to want to warn the user)
     is colorized by the editor itself. In that case, you might want to
     either make a patch or a suggestion to the author of your favorite
     editor's colorization package.

-Peff