Re: [PATCH] userdiff: add built-in pattern for CSS

From: William Duclot <william.duclot@ensimag.grenoble-inp.fr>
Date: May 24, 2016, 22:12 UTC
Message-ID: <925654601.13199.1464127975010.JavaMail.zimbra@ensimag.grenoble-inp.fr>
In-Reply-To: <xmqqmvnf46on.fsf@gitster.mtv.corp.google.com>

> It is not a big deal for a small single-patch topic like this, but
> it often is hard to reviewers if you do not respond to comments you
> received and instead just send a new version of the patch with
> "changes since..." comment.  Please make it a habit to do both.

Apologies, I am not quite used to work with a mailing list yet, I
will make sure to respect this in the future!

Show 5 quoted lines

>  - Have you considered using IPATTERN()?  PATTERNS() that defaults
>    case sensitive match is suitable for real languages with fixed
>    case keywords, but the pattern you are defining for CSS does not
>    do anything special for any set of fixed-case built-in keywords,
>    and appears to be better served by IPATTERN().

I did have considered IPATTERN(), but assumed that case-sensitive was default and case-insensitive was the exception. As the CSS pattern does not deal with letters at all it seemed sensible to me to follow the example of the HTML pattern, which use PATTERNS().

Show 8 quoted lines

>  - In our codebase, we format multi-line comments in a particular
>    way, namely
> 
> 	/*
>          * A multi-line comment begins with slash asterisk
>          * on its own line, and its closing asterisk slash
>          * also is on its own line.
>          */

I take good note of that. I took example on the fortran pattern
comment, should I fix it too while I'm at it?

>  - Try not to write overlong lines.  If your expression needs to
>    become long and there is no good place to fold lines, that is one
>    thing, but an overlong comment is unexcuable, as you can fold
>    lines anywhere between words.

Again, I take good note of that.

Thank you for your time!