threads / patch / 13534

patchUpdated status to show 'Not currently on any branch' in red

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

## tl;dr

3 messages between May 15, 2008 and May 16, 2008. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Chris Parsons· May 15, 2008, 19:37 UTC · lore

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(-)

Show changes to 2 files +6 −2

wt-status.c, wt-status.h

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

Miklos Vajna· May 15, 2008, 22:44 UTC · re: Chris Parsons · lore

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

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. :)

Jeff King· May 16, 2008, 02:05 UTC · re: Chris Parsons · lore

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

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.

Show 8 quoted lines

> 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.

Show 8 quoted lines

>  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