Refactor status producers and make status -s use color
cover Refactor status producers and make status -s use color
This is is the refactoring as suggested by Junio, and rebased on top is the patch which makes status -s obey color.status. I also adjusted the latter to not color the space separating the (colored) status letters from the (uncolored) file name.
This mini series is directly on top of 91691ec (Merge branch 'jk/1.7.0-status' into next, 2009-11-27).
Michael J Gruber (2):
builtin-commit: refactor short-status code into wt-status.c status -s: obey color.status
builtin-commit.c | 105 ++++------------------------------------------------- wt-status.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++ wt-status.h | 2 + 3 files changed, 106 insertions(+), 97 deletions(-)
PATCH 1/2 builtin-commit: refactor short-status code into wt-status.c
Currently, builtin-commit.c contains most code producing the short-status output, whereas wt-status.c contains most of the code for the long format.
Refactor so that most of the long and short format producing code resides in wt-status.c and is named analogously.
--- builtin-commit.c | 101 ++--------------------------------------------------- wt-status.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++ wt-status.h | 2 + 3 files changed, 95 insertions(+), 97 deletions(-)
builtin-commit.c +4 -97
@@ -79,8 +79,6 @@ static enum {STATUS_FORMAT_PORCELAIN , } status_format = STATUS_FORMAT_LONG; -static void short_print (struct wt_status *s,int null_termination); -static int opt_parse_m (const struct option *opt,const char *arg,int unset) {struct strbuf *buf = opt->value;
@@ -387,10 +385,10 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, intswitch (status_format) {case STATUS_FORMAT_SHORT : -short_print (s, null_termination); +wt_shortstatus_print (s, null_termination);break ;case STATUS_FORMAT_PORCELAIN : -short_print (s, null_termination); +wt_shortstatus_print (s, null_termination);break ;case STATUS_FORMAT_LONG :wt_status_print (s);
@@ -982,97 +980,6 @@ static int git_status_config(const char *k, const char *v, void *cb)return git_diff_ui_config (k, v,NULL ); } -#define quote_path quote_path_relative - -static void short_unmerged (int null_termination,struct string_list_item *it, -struct wt_status *s) -{ -struct wt_status_change_data *d = it->util; -const char *how ="??" ; - -switch (d->stagemask) { -case 1 : how ="DD" ;break ;/* both deleted */ -case 2 : how ="AU" ;break ;/* added by us */ -case 3 : how ="UD" ;break ;/* deleted by them */ -case 4 : how ="UA" ;break ;/* added by them */ -case 5 : how ="DU" ;break ;/* deleted by us */ -case 6 : how ="AA" ;break ;/* both added */ -case 7 : how ="UU" ;break ;/* both modified */ - } -printf ("%s " , how); -if (null_termination) { -fprintf (stdout,"%s%c" , it->string,0 ); - }else { -struct strbuf onebuf =STRBUF_INIT ; -const char *one; - one =quote_path (it->string,-1 , &onebuf, s->prefix); -printf ("%s\n" , one); -strbuf_release (&onebuf); - } -} - -static void short_status (int null_termination,struct string_list_item *it, -struct wt_status *s) -{ -struct wt_status_change_data *d = it->util; - -printf ("%c%c " , - !d->index_status ?' ' : d->index_status, - !d->worktree_status ?' ' : d->worktree_status); -if (null_termination) { -fprintf (stdout,"%s%c" , it->string,0 ); -if (d->head_path) -fprintf (stdout,"%s%c" , d->head_path,0 ); - }else { -struct strbuf onebuf =STRBUF_INIT ; -const char *one; -if (d->head_path) { - one =quote_path (d->head_path,-1 , &onebuf, s->prefix); -printf ("%s -> " , one); -strbuf_release (&onebuf); - } - one =quote_path (it->string,-1 , &onebuf, s->prefix); -printf ("%s\n" , one); -strbuf_release (&onebuf); - } -} - -static void short_untracked (int null_termination,struct string_list_item *it, -struct wt_status *s) -{ -if (null_termination) { -fprintf (stdout,"?? %s%c" , it->string,0 ); - }else { -struct strbuf onebuf =STRBUF_INIT ; -const char *one; - one =quote_path (it->string,-1 , &onebuf, s->prefix); -printf ("?? %s\n" , one); -strbuf_release (&onebuf); - } -} - -static void short_print (struct wt_status *s,int null_termination) -{ -int i; -for (i =0 ; i < s->change.nr; i++) { -struct wt_status_change_data *d; -struct string_list_item *it; - - it = &(s->change.items[i]); - d = it->util; -if (d->stagemask) -short_unmerged (null_termination, it, s); -else -short_status (null_termination, it, s); - } -for (i =0 ; i < s->untracked.nr; i++) { -struct string_list_item *it; - - it = &(s->untracked.items[i]); -short_untracked (null_termination, it, s); - } -} -int cmd_status (int argc,const char **argv,const char *prefix) {struct wt_status s;
@@ -1115,10 +1022,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)case STATUS_FORMAT_SHORT :if (s.relative_paths) s.prefix = prefix; -short_print (&s, null_termination); +wt_shortstatus_print (&s, null_termination);break ;case STATUS_FORMAT_PORCELAIN : -short_print (&s, null_termination); +wt_shortstatus_print (&s, null_termination);break ;case STATUS_FORMAT_LONG : s.verbose = verbose;
wt-status.c +89 -0
92 lines not on this page · raw message
wt-status.h +2 -0
@@ -56,4 +56,6 @@ void wt_status_prepare(struct wt_status *s);void wt_status_print (struct wt_status *s);void wt_status_collect (struct wt_status *s); +void wt_shortstatus_print (struct wt_status *s,int null_termination); +#endif /* STATUS_H */
Signature · 1 line
1.6.6.rc1.282.ge6667
Re: [PATCH 1/2] builtin-commit: refactor short-status code into wt-status.c
Quoted 7 lines · Michael J Gruber
Currently,
builtin-commit.ccontains most code producing the short-status output, whereaswt-status.ccontains most of the code for the long format.Refactor so that most of the long and short format producing code resides in
wt-status.cand is named analogously.
Quoted 6 lines
Signed-off-by Michael J Gruber <[email protected]>--- builtin-commit.c | 101 ++--------------------------------------------------- wt-status.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++ wt-status.h | 2 + 3 files changed, 95 insertions(+), 97 deletions(-)
Nice.
Thanks.
PATCH 2/2 status -s: obey color.status
In reply to Michael J GruberMake the short version of status obey the color.status boolean. We color the status letters only, because they carry the state information and are potentially colored differently, such as for a file with staged changes as well as changes in the worktree against the index.
--- builtin-commit.c | 4 ++++ wt-status.c | 21 ++++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-)
builtin-commit.c +4 -0
@@ -1022,6 +1022,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)case STATUS_FORMAT_SHORT :if (s.relative_paths) s.prefix = prefix; +if (s.use_color ==-1 ) + s.use_color = git_use_color_default; +if (diff_use_color_default ==-1 ) + diff_use_color_default = git_use_color_default;wt_shortstatus_print (&s, null_termination);break ;case STATUS_FORMAT_PORCELAIN :
wt-status.c +14 -7
@@ -616,14 +616,14 @@ static void wt_shortstatus_unmerged(int null_termination, struct string_list_itecase 6 : how ="AA" ;break ;/* both added */ case 7 : how ="UU" ;break ;/* both modified */ } -printf ("%s " , how); +color_fprintf (s->fp,color (WT_STATUS_UNMERGED , s),"%s" , how);if (null_termination) { -fprintf (stdout,"%s%c" , it->string,0 ); +fprintf (stdout," %s%c" , it->string,0 ); }else {struct strbuf onebuf =STRBUF_INIT ;const char *one; one =quote_path (it->string,-1 , &onebuf, s->prefix); -printf ("%s\n" , one); +printf (" %s\n" , one);strbuf_release (&onebuf); } }
@@ -633,9 +633,15 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item {struct wt_status_change_data *d = it->util; -printf ("%c%c " , - !d->index_status ?' ' : d->index_status, - !d->worktree_status ?' ' : d->worktree_status); +if (d->index_status) +color_fprintf (s->fp,color (WT_STATUS_UPDATED , s),"%c" , d->index_status); +else +putchar (' ' ); +if (d->worktree_status) +color_fprintf (s->fp,color (WT_STATUS_CHANGED , s),"%c" , d->worktree_status); +else +putchar (' ' ); +putchar (' ' );if (null_termination) {fprintf (stdout,"%s%c" , it->string,0 );if (d->head_path)
@@ -663,7 +669,8 @@ static void wt_shortstatus_untracked(int null_termination, struct string_list_itstruct strbuf onebuf =STRBUF_INIT ;const char *one; one =quote_path (it->string,-1 , &onebuf, s->prefix); -printf ("?? %s\n" , one); +color_fprintf (s->fp,color (WT_STATUS_UNTRACKED , s),"??" ); +printf (" %s\n" , one);strbuf_release (&onebuf); } }
Signature · 1 line
1.6.6.rc1.282.ge6667
Re: [PATCH 2/2] status -s: obey color.status
Quoted 4 lines · Michael J Gruber
Make the short version of status obey the color.status boolean. We color the status letters only, because they carry the state information and are potentially colored differently, such as for a file with staged changes as well as changes in the worktree against the index.
This seems to also turn on color for --porcelain in some cases, because git_status_config unconditionally sets s->use_color if you are using color.status instead of color.ui. I think we are probably best just explicitly disabling options for the "porcelain" format rather than trying to come up with some trickery to make sure they never get set. Like:
-- >8 --
Subject: [PATCH] status: disable color for porcelain format
The porcelain format is identical to the shortstatus format, except that it should not respect any user configuration, including color.
--- builtin-commit.c | 4 ++-- wt-status.c | 6 ++++++ wt-status.h | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)
builtin-commit.c +2 -2
@@ -388,7 +388,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, intwt_shortstatus_print (s, null_termination);break ;case STATUS_FORMAT_PORCELAIN : -wt_shortstatus_print (s, null_termination); +wt_porcelain_print (s, null_termination);break ;case STATUS_FORMAT_LONG :wt_status_print (s);
@@ -1029,7 +1029,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)wt_shortstatus_print (&s, null_termination);break ;case STATUS_FORMAT_PORCELAIN : -wt_shortstatus_print (&s, null_termination); +wt_porcelain_print (&s, null_termination);break ;case STATUS_FORMAT_LONG : s.verbose = verbose;
wt-status.c +6 -0
@@ -696,3 +696,9 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination)wt_shortstatus_untracked (null_termination, it, s); } } + +void wt_porcelain_print (struct wt_status *s,int null_termination) +{ + s->use_color =0 ; +wt_shortstatus_print (s, null_termination); +}
wt-status.h +1 -0
@@ -57,5 +57,6 @@ void wt_status_print(struct wt_status *s);void wt_status_collect (struct wt_status *s);void wt_shortstatus_print (struct wt_status *s,int null_termination); +void wt_porcelain_print (struct wt_status *s,int null_termination);#endif /* STATUS_H */
Signature · 1 line
1.6.6.rc1.292.gd8fe.dirty
Re: [PATCH 2/2] status -s: obey color.status
Quoted 5 lines · Jeff King
This seems to also turn on color for --porcelain in some cases, because
git_status_configunconditionally setss->use_colorif you are using color.status instead of color.ui. I think we are probably best just explicitly disabling options for the "porcelain" format rather than trying to come up with some trickery to make sure they never get set.
Also, this means we can hoist repeated code out of the switch statement, like this:
-- >8 --
Subject: [PATCH] status: reduce duplicated setup code
We have three output formats: short, porcelain, and long. The short and long formats respect user-config, and the porcelain one does not. This led to us repeating config-related setup code for the short and long formats.
Since the last commit, color config is explicitly cleared when showing the porcelain format. Let's do the same with relative-path configuration, which enables us to hoist the duplicated code from the switch statement in cmd_status.
As a bonus, this fixes "commit --dry-run --porcelain", which was unconditionally setting up that configuration, anyway.
--- builtin-commit.c | 19 +++++++------------ wt-status.c | 2 ++ 2 files changed, 9 insertions(+), 12 deletions(-)
builtin-commit.c +7 -12
@@ -1018,14 +1018,15 @@ int cmd_status(int argc, const char **argv, const char *prefix) s.is_initial =get_sha1 (s.reference, sha1) ?1 :0 ;wt_status_collect (&s); +if (s.relative_paths) + s.prefix = prefix; +if (s.use_color ==-1 ) + s.use_color = git_use_color_default; +if (diff_use_color_default ==-1 ) + diff_use_color_default = git_use_color_default; +switch (status_format) {case STATUS_FORMAT_SHORT : -if (s.relative_paths) - s.prefix = prefix; -if (s.use_color ==-1 ) - s.use_color = git_use_color_default; -if (diff_use_color_default ==-1 ) - diff_use_color_default = git_use_color_default;wt_shortstatus_print (&s, null_termination);break ;case STATUS_FORMAT_PORCELAIN :
@@ -1033,12 +1034,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)break ;case STATUS_FORMAT_LONG : s.verbose = verbose; -if (s.relative_paths) - s.prefix = prefix; -if (s.use_color ==-1 ) - s.use_color = git_use_color_default; -if (diff_use_color_default ==-1 ) - diff_use_color_default = git_use_color_default;wt_status_print (&s);break ; }
wt-status.c +2 -0
@@ -700,5 +700,7 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination)void wt_porcelain_print (struct wt_status *s,int null_termination) { s->use_color =0 ; + s->relative_paths =0 ; + s->prefix =NULL ;wt_shortstatus_print (s, null_termination); }
Signature · 1 line
1.6.6.rc1.292.gd8fe.dirty
Re: [PATCH 2/2] status -s: obey color.status
In reply to Jeff KingJeff King venit, vidit, dixit 07.12.2009 06:17:
Quoted 7 lines · Michael J Gruber
Quoted 5 lines
Make the short version of status obey the color.status boolean. We color the status letters only, because they carry the state information and are potentially colored differently, such as for a file with staged changes as well as changes in the worktree against the index.
Quoted 6 lines
This seems to also turn on color for --porcelain in some cases, because
git_status_configunconditionally setss->use_colorif you are using color.status instead of color.ui. I think we are probably best just explicitly disabling options for the "porcelain" format rather than trying to come up with some trickery to make sure they never get set. Like:
Thanks. I let myself get fooled by the apparent option handling within the switch statement (which is OK for relativePaths) (and some subconscious link between null_termination and porcelain, which goes one way only).
I guess this shows (again) that one needs tests for everything... If I get around to I'll amend t$(relevantoneicantrecallrightnow) with tests for long and short status with color and porcelain with the various options.
Michael