Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Aug 28, 2013, 20:59 UTC
- Message-ID
- <vpq61up5y5s.fsf@anie.imag.fr>
- In-Reply-To
- <xmqq1u5da8dp.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> Matthieu Moy <Matthieu.Moy@imag.fr> writes: > >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index ec57a15..dacf4b9 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -2118,6 +2118,11 @@ status.branch:: >> Set to true to enable --branch by default in linkgit:git-status[1]. >> The option --no-branch takes precedence over this variable. >> >> +status.displayCommentChar:: >> + If set to false, linkgit:git-status[1] will not prefix each >> + line with the comment char (`core.commentchar`, i.e. `#` by >> + default). Defaults to true. > > We prefix core.commentchar followed by a single space for non-empty > lines; is that worth mentioning, I wonder?
(and sometimes # is followed by a tab)
I don't think it's worth the trouble. The goal is not to document the format precisely, but to explain the user how to use the variable.
> Also I envision that we would extend core.commentchar to be more > than a single character. Is "displayCommentChar" still a good name > for this setting?
It is as good as core.commentChar ;-).
I chose the variable name to remind commentChar (after finding commentChar, one can grep it and find status.displayCommentChar). I tend to think that it's better than omitCommentPrefix, but I can change is people think it's better.
> What are our plans to help existing scripts people have written over > time, especially before "status -s" was invented, that will be > broken by use of this?
I don't know what's the best plan, but I think any sensible plan starts by waiting for a few releases, so that Git version not implementing status.displayCommentChar become rare enough. So we can delay the actual plan until next year I guess.
That said, I had an idea that may help the transition: allow "auto" as a value, just like we did for colors. A patch implementing this (on top of the previous series) is below. Good point: "auto" is a safe value, as it won't break scripts. There is one drawback, though: "auto" is not a really good default value in the long run, since newcommers may wonder why there are differences between "git status" and "git status | cat".
I still find this tempting, and it would make the transition so much easier. I would even argue to flip the default to "auto" for Git 2.0 then (after all, we didn't even wait for this to change color.ui).
Maybe, later, a transition from "auto" to "never" could be done. Or perhaps it's not so terrible to keep auto (my "ls" and "ls | cat" produce very different outputs, and it never bugged me).
diff --git a/builtin/commit.c b/builtin/commit.c index d4cfced..00e9487 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -163,6 +163,21 @@ static void determine_whence(struct wt_status *s) s->whence = whence; } +static void determine_comment_mode(struct wt_status *s) +{ + if (s->display_comment_char == COMMENT_AUTO) { + /* + * determine_comment_mode is used only for cmd_status, + * which always prints to stdout. + */ + if (isatty(1) || pager_in_use()) { + s->display_comment_char = COMMENT_NEVER; + } else { + s->display_comment_char = COMMENT_ALWAYS; + } + } +} + static void rollback_index_files(void) { switch (commit_style) { @@ -1183,6 +1198,20 @@ static int git_status_config(const char *k, const char *v, void *cb) return 0; } if (!strcmp(k, "status.displaycommentchar")) { + if (v) { + if (!strcasecmp(v, "never")) { + s->display_comment_char = COMMENT_NEVER; + return 0; + } + if (!strcasecmp(v, "always")) { + s->display_comment_char = COMMENT_ALWAYS; + return 0; + } + if (!strcasecmp(v, "auto")) { + s->display_comment_char = COMMENT_AUTO; + return 0; + } + } s->display_comment_char = git_config_bool(k, v); return 0; } @@ -1254,6 +1283,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) gitmodules_config(); git_config(git_status_config, &s); determine_whence(&s); + determine_comment_mode(&s); argc = parse_options(argc, argv, prefix, builtin_status_options, builtin_status_usage, 0); @@ -1504,7 +1534,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) * Ignore status.displayCommentChar: we do need comments in * COMMIT_EDITMSG. */ - s.display_comment_char = 1; + s.display_comment_char = COMMENT_ALWAYS; determine_whence(&s); s.colopts = 0; diff --git a/wt-status.h b/wt-status.h index ac02c64..dbc706e 100644 --- a/wt-status.h +++ b/wt-status.h @@ -40,6 +40,12 @@ struct wt_status_change_data { unsigned new_submodule_commits : 1; }; +enum comment_mode { + COMMENT_NEVER = 0, + COMMENT_ALWAYS = 1, + COMMENT_AUTO +}; + struct wt_status { int is_initial; char *branch; @@ -50,7 +56,7 @@ struct wt_status { enum commit_whence whence; int nowarn; int use_color; - int display_comment_char; + enum comment_mode display_comment_char; int relative_paths; int submodule_summary; int show_ignored_files;
-- Matthieu Moy http://www-verimag.imag.fr/~moy/