patchRe: [PATCH] userdiff: remove empty subexpression from elixir regex

The regex failed to compile on FreeBSD.

Fixes: a807200f67588f6e
Signed-off-by: Ed Maste <emaste@FreeBSD.org>
---
 userdiff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Fri, Dec 13, 2019 at 05:39:02PM +0000, Ed Maste wrote:

It took me a minute to see why this was different than the similar "Numbers" line below. The issue is the comma at the end of the previous line; this is starting a new string, whereas the "Numbers" line is pasting to the existing string.

And that is the right thing, since these strings are the funcname and word_regex patterns, respectively.

So I think this is the correct fix. Many of the other regexes in this list use "/* -- */" to seperate the two for readability. Maybe worth doing here, too?

-Peff

On Fri, 13 Dec 2019 at 12:45, Jeff King <peff@peff.net> wrote:

Yeah, this elixir set seems to be the only one with comments on the individual subexpressions in the second set but the extra /* -- */ does make it a bit more clear. Patch v2 sent.

The regex failed to compile on FreeBSD.

Fixes: a807200f67588f6e
Signed-off-by: Ed Maste <emaste@FreeBSD.org>
---
Add /* -- */ to make things more clear and be consistent with other
patterns.

 userdiff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

On Fri, Dec 13, 2019 at 05:55:35PM +0000, Ed Maste wrote:

Thanks, this looks good to me.

-Peff

Am 13.12.19 um 18:55 schrieb Ed Maste:

Having a references is this form is unusual for our codebase. (Not that I mind a lot, though.) I expect that Junio will commit the fix on top of the commit that introduced the bogus regex anyway (branch ln/userdiff-elixir), and then it will be easy find.

This text would be nice to have in the commit message.

Good catch!

Tested-by: Johannes Sixt <j6t@kdbg.org>

Thanks!

-- Hannes

On Fri, 13 Dec 2019 at 14:24, Johannes Sixt <j6t@kdbg.org> wrote:

Ok, I picked this up from the Linux kernel where someone added a
Fixes: tag to one of my changes (which had the hash of the original
change as part of the commit message body).

Ah, I didn't think it was remarkable (it's consistent with all of the existing entries) but the change is indeed broader than what the commit message implies. I'm happy to send a v3 with an amended commit message if that's desired.

Ed Maste <emaste@freebsd.org> writes:

Let's save one round-trip, then. Here is what I will queue on the 'pu' branch.

Thanks, all.

-- >8 --
From: Ed Maste <emaste@FreeBSD.org>
Date: Fri, 13 Dec 2019 17:55:35 +0000
Subject: [PATCH] userdiff: remove empty subexpression from elixir regex

The regex failed to compile on FreeBSD.

Also add /* -- */ mark to separate the two regex entries given to the PATTERNS() macro, to make it consistent with patterns for other content types.

Signed-off-by: Ed Maste <emaste@FreeBSD.org>
Reviewed-by: Jeff King <peff@peff.net>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 userdiff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Nothing to do with the patch from Ed, but the regex following his correction matches a lot of things that decidedly are not "Numbers with specific bases" as it claims to do in the comment.

Ed Maste writes:

Here, things like "+0bad" would match as a base 2 number, which doesn't seem right. If it's intended to match that broadly, I'd have expected a comment to that effect. Maybe something like

"|[-+]?0b[01]+|[-+]?0o[0-7]+|[-+]?0x[0-9a-fA-F]+"

or (if the resulting group is not a problem someplace else)

"|[-+]?0(b[01]+|o[0-7]+|x[0-9a-fA-F]+)"

to more specifically match only what the comment says?

Regards, Achim.

Achim Gratz <Stromeko@nexgo.de> writes:

No need for such a comment, as it is implicit that we assume the user writes reasonable text that our patterns try to match.