Re: [PATCH 2/2] commit: allow core.commentChar=auto for character auto selection

From: Duy Nguyen <pclouds@gmail.com>
Date: May 16, 2014, 23:41 UTC
Message-ID: <CACsJy8ASmBV2ORZeDQ=GUAGcq80_Gz2zmv6MGysFAVNR4Hs3YA@mail.gmail.com>
In-Reply-To: <20140516164053.GB12314@google.com>

On Fri, May 16, 2014 at 11:40 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 11 quoted lines

> Nguyễn Thái Ngọc Duy wrote:
>
>> core.commentChar starts with '#' as in default but if it's already in
>> the prepared message, find another one among a small subset. This
>> should stop surprises because git strips some lines unexpectedly.
>
> Probably worth mentioning this only kicks in if someone explicitly
> configures [core] commentchar = auto.
>
> Would it be a goal to make 'auto' the default eventually if people
> turn out to like it?

No. I started this with a patch that does this automatically without a config knob. It broke git-commit with custom templates. It broke --cleanup=strip -e -F.. So people may want to put this in ~/.gitconfig but it's their decision.

Show 11 quoted lines

>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -594,6 +594,40 @@ static char *cut_ident_timestamp_part(char *string)
>>       return ket;
>>  }
>>
>> +static void adjust_comment_line_char(const struct strbuf *sb)
>> +{
>> +     char candidates[] = " @!#$%^&|:;~";
>
> This prefers '@' over '#'.  Intended?

It used to be the order of keys 1, 2, 3... on qwerty keyboard, but I was afraid ! may become history expansion in bash so I made @ preferred over !. Will make # and ; higher priority.

Show 17 quoted lines

>> +     char *candidate;
>> +     const char *p;
>> +
>> +     if (!sb->len)
>> +             return;
>> +
>> +     if (!strchr(candidates, comment_line_char))
>> +             candidates[0] = comment_line_char;
>
> Could do
>
>         if (!memchr(sb->buf, comment_line_char, sb->len))
>                 return;
>
> to solve the precedence problem.  The comment_line_char not appearing
> in the message is the usual case and handling it separately means it
> gets handled faster.

Now that we use "auto" to turn this on, the placeholder candidates[0] could probably be removed, we know comment_line_char is '#' at this point.

Show 13 quoted lines

>> --- a/config.c
>> +++ b/config.c
>> @@ -829,6 +829,8 @@ static int git_default_core_config(const char *var, const char *value)
>>               if (!ret) {
>>                       if (comment[0] && !comment[1])
>>                               comment_line_char = comment[0];
>> +                     else if (!strcasecmp(comment, "auto"))
>> +                             auto_comment_line_char = 1;
>
> Is there a way to disable 'auto' if 'auto' is already set?
>
> comment_line_char still can be set and matters when 'auto' is set.
> Should they be separate settings?

I think the next core.commentChar should override the old one, so auto_comment_line_char should be clear when we set new value to comment_line_char.

-- 
Duy