Junio C Hamano <gits...@pobox.com> writes:

> Matthieu Moy <matthieu....@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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to