{"thread":{"id":"13534","subject":"[PATCH] Updated status to show 'Not currently on any branch' in red","startedAt":"2008-05-15T19:37:52Z","lastAt":"2008-05-16T02:05:42Z","messageCount":3,"participants":["Chris Parsons","Miklos Vajna","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"77065","messageId":"612A0CE5-9482-4D27-B2E4-D55F7511A1C2@edendevelopment.co.uk","threadId":"13534","inReplyTo":null,"subject":"[PATCH] Updated status to show 'Not currently on any branch' in red","fromName":"Chris Parsons","fromEmail":"chris@edendevelopment.co.uk","sentAt":"2008-05-15T19:37:52Z","receivedAt":"2008-05-15T19:37:52Z","isPatch":true,"sender":{"key":"chris@edendevelopment.co.uk","avatar":null},"body":"Hi all, first post and patch, please be gentle :)\n\nI'm a fairly new user to git and have more than once been burnt by \nchecking out an arbitrary commit and then checking in code on top of \nthat commit. If you realise your mistake quickly, you can reset and \nreapply the commit to a branch, but if you checkout another branch \nyour commit can disappear and become hard to find.\n\nTherefore as a help to new users I've turned the 'Not currently on any \nbranch' red on 'git status' so that it's harder to miss the fact.\n\nPerhaps git should not allow commits in this case? I'm too much of a \nnovice to know whether that's ever needed, but perhaps someone has a \ngood reason for allowing it.\n\nThanks and regards\nChris\n\nSigned-off-by: Chris Parsons \n---\n wt-status.c | 7 +++++--\n wt-status.h | 1 +\n 2 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex a44c543..42a1ff8 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -18,6 +18,7 @@ static char wt_status_colors[][COLOR_MAXLEN] = {\n \t\"\\033[32m\", /* WT_STATUS_UPDATED: green */\n \t\"\\033[31m\", /* WT_STATUS_CHANGED: red */\n \t\"\\033[31m\", /* WT_STATUS_UNTRACKED: red */\n+\t\"\\033[31m\", /* WT_STATUS_NOBRANCH: red */\n };\n\n static const char use_add_msg[] =\n@@ -315,7 +316,8 @@ void wt_status_print(struct wt_status *s)\n {\n \tunsigned char sha1[20];\n \ts->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;\n-\n+ const char* branch_color = color(WT_STATUS_HEADER);\n+\n \tif (s->branch) {\n \t\tconst char *on_what = \"On branch \";\n \t\tconst char *branch_name = s->branch;\n@@ -323,9 +325,10 @@ void wt_status_print(struct wt_status *s)\n \t\t\tbranch_name += 11;\n \t\telse if (!strcmp(branch_name, \"HEAD\")) {\n \t\t\tbranch_name = \"\";\n+\t\t\tbranch_color = color(WT_STATUS_NOBRANCH);\n \t\t\ton_what = \"Not currently on any branch.\";\n \t\t}\n-\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER),\n+\t\tcolor_fprintf_ln(s->fp, branch_color,\n \t\t\t\"# %s%s\", on_what, branch_name);\n \t}\n\ndiff --git a/wt-status.h b/wt-status.h\nindex 7d61410..f0675fd 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -8,6 +8,7 @@ enum color_wt_status {\n \tWT_STATUS_UPDATED,\n \tWT_STATUS_CHANGED,\n \tWT_STATUS_UNTRACKED,\n+\tWT_STATUS_NOBRANCH,\n };\n\n struct wt_status {\n-- \n1.5.5.1.249.g3fdb.dirty\n"},{"id":"77079","messageId":"20080515224436.GQ27724@genesis.frugalware.org","threadId":"13534","inReplyTo":"612A0CE5-9482-4D27-B2E4-D55F7511A1C2@edendevelopment.co.uk","subject":"Re: [PATCH] Updated status to show 'Not currently on any branch' in red","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-05-15T22:44:36Z","receivedAt":"2008-05-15T22:44:36Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, May 15, 2008 at 08:37:52PM +0100, Chris Parsons wrote:\n> Hi all, first post and patch, please be gentle :)\n\nIn general, please write such comments after the --- and before the\ndiffstat, so it won't be part of the commit message. More tricks in\nDocumentation/SubmittingPatches. :)\n"},{"id":"77092","messageId":"20080516020541.GA14727@sigill.intra.peff.net","threadId":"13534","inReplyTo":"612A0CE5-9482-4D27-B2E4-D55F7511A1C2@edendevelopment.co.uk","subject":"Re: [PATCH] Updated status to show 'Not currently on any branch' in red","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-16T02:05:42Z","receivedAt":"2008-05-16T02:05:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 15, 2008 at 08:37:52PM +0100, Chris Parsons wrote:\n\n> Hi all, first post and patch, please be gentle :)\n\nHi, and welcome.\n\nBut as Miklos pointed out, please separate \"cover letter\" material from\nthe commit message by putting it under the \"---\" cut.\n\n> I'm a fairly new user to git and have more than once been burnt by \n> checking out an arbitrary commit and then checking in code on top of that \n> commit. If you realise your mistake quickly, you can reset and reapply the \n> commit to a branch, but if you checkout another branch your commit can \n> disappear and become hard to find.\n>\n> Therefore as a help to new users I've turned the 'Not currently on any \n> branch' red on 'git status' so that it's harder to miss the fact.\n\nI think this is definitely a good change overall, but I have some\nspecific comments on the implementation below.\n\n> Perhaps git should not allow commits in this case? I'm too much of a \n> novice to know whether that's ever needed, but perhaps someone has a good \n> reason for allowing it.\n\nThere was much discussion of this when the detached HEAD feature was\nintroduced, but it was decided that allowing commits was useful. I think\nputting the text in red is a nice way to highlight a potential mistake,\nthough.\n\n> static const char use_add_msg[] =\n> @@ -315,7 +316,8 @@ void wt_status_print(struct wt_status *s)\n> {\n> \tunsigned char sha1[20];\n> \ts->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;\n> -\n> + const char* branch_color = color(WT_STATUS_HEADER);\n> +\n\nIndentation?\n\n> -\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER),\n> +\t\tcolor_fprintf_ln(s->fp, branch_color,\n> \t\t\t\"# %s%s\", on_what, branch_name);\n\nThis makes the whole line red; I think it would look nicer matching the\nred / green files last on in the file list by coloring just the message\npart. IOW,\n\n color_fprintf(s->fp, color(WT_STATUS_HEADER), \"# \");\n color_fprintf_ln(s->fp, branch_color, %s%s\", on_what, branch_name);\n\n\nAnd two final points on missing items:\n\n 1. This should probably be configurable. You'd need to update\n parse_status_slot, as well as the documentation in config.txt.\n\n 2. Note that this just covers the colorization of status sent to the\n terminal or pager. What is seen in the editor while writing the\n commit message (which is a likely place to want to warn the user)\n is colorized by the editor itself. In that case, you might want to\n either make a patch or a suggestion to the author of your favorite\n editor's colorization package.\n\n-Peff\n"}]}