Re: Re* [PATCH v4] userdiff: improve java hunk header regex

From: Johannes Sixt <j6t@kdbg.org>
Date: Aug 11, 2021, 07:14 UTC
Message-ID: <157a0c35-1c82-9a2e-3bcd-ae6059ec71bd@kdbg.org>
In-Reply-To: <xmqq35rhc5la.fsf_-_@gitster.g>

Am 11.08.21 um 00:12 schrieb Junio C Hamano:

Show 32 quoted lines

> Subject: userdiff: comment on the builtin patterns
> 
> Remind developers that they do not need to go overboard to implement
> patterns to prepare for invalid constructs.  They only have to be
> sufficiently permissive, assuming that the payload is syntactically
> correct.
> 
> Text stolen mostly from Johannes Sixt.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  userdiff.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git c/userdiff.c w/userdiff.c
> index d9b2ba752f..1a6d27fda6 100644
> --- c/userdiff.c
> +++ w/userdiff.c
> @@ -13,6 +13,16 @@ static int drivers_alloc;
>  #define IPATTERN(name, pattern, word_regex)			\
>  	{ name, NULL, -1, { pattern, REG_EXTENDED | REG_ICASE }, \
>  	  word_regex "|[^[:space:]]|[\xc0-\xff][\x80-\xbf]+" }
> +
> +/*
> + * Built-in drivers for various languages, sorted by their names
> + * (except that the "default" is left at the end).
> + *
> + * When writing or updating patterns, assume that the contents these
> + * patterns are applied to are syntactically correct.  You do not have
> + * to implement all syntactical corner cases---the patterns have to be
> + * sufficiently permissive.
> + */

IMO, as written, the comment falls short of suggesting that patterns can be simple. How about appending "and can be simple"?

>  static struct userdiff_driver builtin_drivers[] = {
>  IPATTERN("ada",
>  	 "!^(.*[ \t])?(is[ \t]+new|renames|is[ \t]+separate)([ \t].*)?$\n"
>