{"thread":{"id":"52450","subject":"Re: [PATCH] userdiff: remove empty subexpression from elixir regex","startedAt":"2019-12-13T20:39:52Z","lastAt":"2019-12-13T22:00:32Z","messageCount":10,"participants":["Jeff King","Ed Maste","Johannes Sixt","Junio C Hamano","Achim Gratz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"388130","messageId":"20191213173902.71541-1-emaste@FreeBSD.org","threadId":"52450","inReplyTo":null,"subject":"[PATCH] userdiff: remove empty subexpression from elixir regex","fromName":"Ed Maste","fromEmail":"emaste@freebsd.org","sentAt":"2019-12-13T17:39:02Z","receivedAt":"2019-12-13T20:39:52Z","isPatch":true,"sender":{"key":"emaste@freebsd.org","avatar":"https://avatars.githubusercontent.com/u/1034582?v=4"},"body":"The regex failed to compile on FreeBSD.\n\nFixes: a807200f67588f6e\nSigned-off-by: Ed Maste \n---\n userdiff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/userdiff.c b/userdiff.c\nindex 324916f20f..165d7e8653 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -35,7 +35,7 @@ PATTERNS(\"dts\",\n PATTERNS(\"elixir\",\n \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n \t /* Atoms, names, and module attributes */\n-\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n+\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n \t /* Numbers with specific base */\n \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n \t /* Numbers */\n-- \n2.24.0\n\n"},{"id":"388129","messageId":"20191213174542.GB117158@coredump.intra.peff.net","threadId":"52450","inReplyTo":"20191213173902.71541-1-emaste@FreeBSD.org","subject":"Re: [PATCH] userdiff: remove empty subexpression from elixir regex","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-12-13T17:45:42Z","receivedAt":"2019-12-13T20:39:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 13, 2019 at 05:39:02PM +0000, Ed Maste wrote:\n\n> diff --git a/userdiff.c b/userdiff.c\n> index 324916f20f..165d7e8653 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -35,7 +35,7 @@ PATTERNS(\"dts\",\n> PATTERNS(\"elixir\",\n> \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n> \t /* Atoms, names, and module attributes */\n> -\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n> +\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n> \t /* Numbers with specific base */\n> \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n> \t /* Numbers */\n\nIt took me a minute to see why this was different than the similar\n\"Numbers\" line below. The issue is the comma at the end of the previous\nline; this is starting a new string, whereas the \"Numbers\" line is\npasting to the existing string.\n\nAnd that is the right thing, since these strings are the funcname and\nword_regex patterns, respectively.\n\nSo I think this is the correct fix. Many of the other regexes in this\nlist use \"/* -- */\" to seperate the two for readability. Maybe worth\ndoing here, too?\n\n-Peff\n"},{"id":"388134","messageId":"20191213175535.87725-1-emaste@FreeBSD.org","threadId":"52450","inReplyTo":"20191213173902.71541-1-emaste@FreeBSD.org","subject":"[PATCH v2] userdiff: remove empty subexpression from elixir regex","fromName":"Ed Maste","fromEmail":"emaste@freebsd.org","sentAt":"2019-12-13T17:55:35Z","receivedAt":"2019-12-13T20:40:05Z","isPatch":true,"sender":{"key":"emaste@freebsd.org","avatar":"https://avatars.githubusercontent.com/u/1034582?v=4"},"body":"The regex failed to compile on FreeBSD.\n\nFixes: a807200f67588f6e\nSigned-off-by: Ed Maste \n---\nAdd /* -- */ to make things more clear and be consistent with other\npatterns.\n\n userdiff.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/userdiff.c b/userdiff.c\nindex 324916f20f..efbe05e5a5 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -34,8 +34,9 @@ PATTERNS(\"dts\",\n \t \"|[-+*/%&^|!~]|>>|<<|&&|\\\\|\\\\|\"),\n PATTERNS(\"elixir\",\n \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n+\t /* -- */\n \t /* Atoms, names, and module attributes */\n-\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n+\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n \t /* Numbers with specific base */\n \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n \t /* Numbers */\n-- \n2.24.0\n\n"},{"id":"388135","messageId":"CAPyFy2DfhVwEFen2G4oOdQS2uo_L=V5gyrpPWUB0uRxNSnWcuQ@mail.gmail.com","threadId":"52450","inReplyTo":"20191213174542.GB117158@coredump.intra.peff.net","subject":"Re: [PATCH] userdiff: remove empty subexpression from elixir regex","fromName":"Ed Maste","fromEmail":"emaste@freebsd.org","sentAt":"2019-12-13T14:11:30Z","receivedAt":"2019-12-13T20:40:06Z","isPatch":true,"sender":{"key":"emaste@freebsd.org","avatar":"https://avatars.githubusercontent.com/u/1034582?v=4"},"body":"On Fri, 13 Dec 2019 at 12:45, Jeff King wrote:\n>\n> And that is the right thing, since these strings are the funcname and\n> word_regex patterns, respectively.\n>\n> So I think this is the correct fix. Many of the other regexes in this\n> list use \"/* -- */\" to seperate the two for readability. Maybe worth\n> doing here, too?\n\nYeah, this elixir set seems to be the only one with comments on the\nindividual subexpressions in the second set but the extra /* -- */\ndoes make it a bit more clear. Patch v2 sent.\n"},{"id":"388138","messageId":"20191213181830.GA122626@coredump.intra.peff.net","threadId":"52450","inReplyTo":"20191213175535.87725-1-emaste@FreeBSD.org","subject":"Re: [PATCH v2] userdiff: remove empty subexpression from elixir regex","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-12-13T18:18:30Z","receivedAt":"2019-12-13T20:40:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 13, 2019 at 05:55:35PM +0000, Ed Maste wrote:\n\n> The regex failed to compile on FreeBSD.\n> \n> Fixes: a807200f67588f6e\n> Signed-off-by: Ed Maste \n> ---\n> Add /* -- */ to make things more clear and be consistent with other\n> patterns.\n\nThanks, this looks good to me.\n\n-Peff\n"},{"id":"388146","messageId":"0c9d891e-382f-03d1-bcbd-d652f1d58f4d@kdbg.org","threadId":"52450","inReplyTo":"20191213175535.87725-1-emaste@FreeBSD.org","subject":"Re: [PATCH v2] userdiff: remove empty subexpression from elixir regex","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-12-13T19:24:47Z","receivedAt":"2019-12-13T20:41:01Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.12.19 um 18:55 schrieb Ed Maste:\n> The regex failed to compile on FreeBSD.\n> \n> Fixes: a807200f67588f6e\n\nHaving a references is this form is unusual for our codebase. (Not that\nI mind a lot, though.) I expect that Junio will commit the fix on top of\nthe commit that introduced the bogus regex anyway (branch\nln/userdiff-elixir), and then it will be easy find.\n\n> Signed-off-by: Ed Maste \n> ---\n> Add /* -- */ to make things more clear and be consistent with other\n> patterns.\n\nThis text would be nice to have in the commit message.\n\n> \n> userdiff.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n> \n> diff --git a/userdiff.c b/userdiff.c\n> index 324916f20f..efbe05e5a5 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -34,8 +34,9 @@ PATTERNS(\"dts\",\n> \t \"|[-+*/%&^|!~]|>>|<<|&&|\\\\|\\\\|\"),\n> PATTERNS(\"elixir\",\n> \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n> +\t /* -- */\n> \t /* Atoms, names, and module attributes */\n> -\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n> +\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n> \t /* Numbers with specific base */\n> \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n> \t /* Numbers */\n> \n\nGood catch!\n\nTested-by: Johannes Sixt \n\nThanks!\n\n-- Hannes\n"},{"id":"388149","messageId":"CAPyFy2B_P7qJ+ocg8rzNWEZWo2uKzaZsfYRvvhwUbAXv2AB6pg@mail.gmail.com","threadId":"52450","inReplyTo":"0c9d891e-382f-03d1-bcbd-d652f1d58f4d@kdbg.org","subject":"Re: [PATCH v2] userdiff: remove empty subexpression from elixir regex","fromName":"Ed Maste","fromEmail":"emaste@freebsd.org","sentAt":"2019-12-13T15:58:40Z","receivedAt":"2019-12-13T20:41:04Z","isPatch":true,"sender":{"key":"emaste@freebsd.org","avatar":"https://avatars.githubusercontent.com/u/1034582?v=4"},"body":"On Fri, 13 Dec 2019 at 14:24, Johannes Sixt wrote:\n>\n> Am 13.12.19 um 18:55 schrieb Ed Maste:\n> > The regex failed to compile on FreeBSD.\n> >\n> > Fixes: a807200f67588f6e\n>\n> Having a references is this form is unusual for our codebase. (Not that\n> I mind a lot, though.) I expect that Junio will commit the fix on top of\n> the commit that introduced the bogus regex anyway (branch\n> ln/userdiff-elixir), and then it will be easy find.\n\nOk, I picked this up from the Linux kernel where someone added a\nFixes: tag to one of my changes (which had the hash of the original\nchange as part of the commit message body).\n\n> > Signed-off-by: Ed Maste \n> > ---\n> > Add /* -- */ to make things more clear and be consistent with other\n> > patterns.\n>\n> This text would be nice to have in the commit message.\n\nAh, I didn't think it was remarkable (it's consistent with all of the\nexisting entries) but the change is indeed broader than what the\ncommit message implies. I'm happy to send a v3 with an amended commit\nmessage if that's desired.\n"},{"id":"388161","messageId":"xmqqzhfwht40.fsf@gitster-ct.c.googlers.com","threadId":"52450","inReplyTo":"CAPyFy2B_P7qJ+ocg8rzNWEZWo2uKzaZsfYRvvhwUbAXv2AB6pg@mail.gmail.com","subject":"Re: [PATCH v2] userdiff: remove empty subexpression from elixir regex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-13T20:23:59Z","receivedAt":"2019-12-13T20:41:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ed Maste writes:\n\n>> > Add /* -- */ to make things more clear and be consistent with other\n>> > patterns.\n>>\n>> This text would be nice to have in the commit message.\n>\n> Ah, I didn't think it was remarkable (it's consistent with all of the\n> existing entries) but the change is indeed broader than what the\n> commit message implies. I'm happy to send a v3 with an amended commit\n> message if that's desired.\n\nLet's save one round-trip, then. Here is what I will queue on the\n'pu' branch.\n\nThanks, all.\n\n-- >8 --\nFrom: Ed Maste \nDate: Fri, 13 Dec 2019 17:55:35 +0000\nSubject: [PATCH] userdiff: remove empty subexpression from elixir regex\n\nThe regex failed to compile on FreeBSD.\n\nAlso add /* -- */ mark to separate the two regex entries given to\nthe PATTERNS() macro, to make it consistent with patterns for other\ncontent types.\n\nSigned-off-by: Ed Maste \nReviewed-by: Jeff King \nHelped-by: Johannes Sixt \nSigned-off-by: Junio C Hamano \n---\n userdiff.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/userdiff.c b/userdiff.c\nindex 577053c10a..0eb34bcd76 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -34,8 +34,9 @@ PATTERNS(\"dts\",\n \t \"|[-+*/%&^|!~]|>>|<<|&&|\\\\|\\\\|\"),\n PATTERNS(\"elixir\",\n \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n+\t /* -- */\n \t /* Atoms, names, and module attributes */\n-\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n+\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n \t /* Numbers with specific base */\n \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n \t /* Numbers */\n-- \n2.24.1-664-g198078bb5a\n\n\n\n"},{"id":"388163","messageId":"87tv64ymam.fsf@Rainer.invalid","threadId":"52450","inReplyTo":"20191213173902.71541-1-emaste@FreeBSD.org","subject":"Numbers with specific base (was: [PATCH] userdiff: remove empty subexpression from elixir regex)","fromName":"Achim Gratz","fromEmail":"stromeko@nexgo.de","sentAt":"2019-12-13T20:59:13Z","receivedAt":"2019-12-13T20:59:30Z","isPatch":true,"sender":{"key":"stromeko@nexgo.de","avatar":null},"body":"\nNothing to do with the patch from Ed, but the regex following his\ncorrection matches a lot of things that decidedly are not \"Numbers with\nspecific bases\" as it claims to do in the comment.\n\nEd Maste writes:\n> PATTERNS(\"elixir\",\n> \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n> \t /* Atoms, names, and module attributes */\n> -\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n> +\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n> \t /* Numbers with specific base */\n> \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n\nHere, things like \"+0bad\" would match as a base 2 number, which doesn't\nseem right. If it's intended to match that broadly, I'd have expected a\ncomment to that effect. Maybe something like\n\n\"|[-+]?0b[01]+|[-+]?0o[0-7]+|[-+]?0x[0-9a-fA-F]+\"\n\nor (if the resulting group is not a problem someplace else)\n\n\"|[-+]?0(b[01]+|o[0-7]+|x[0-9a-fA-F]+)\"\n\nto more specifically match only what the comment says?\n\n\n\nRegards,\nAchim.\n-- \n+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+\n\nSD adaptation for Waldorf rackAttack V1.04R1:\nhttp://Synth.Stromeko.net/Downloads.html#WaldorfSDada\n\n"},{"id":"388177","messageId":"xmqqimmjhon9.fsf@gitster-ct.c.googlers.com","threadId":"52450","inReplyTo":"87tv64ymam.fsf@Rainer.invalid","subject":"Re: Numbers with specific base","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-13T22:00:26Z","receivedAt":"2019-12-13T22:00:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Achim Gratz writes:\n\n> Nothing to do with the patch from Ed, but the regex following his\n> correction matches a lot of things that decidedly are not \"Numbers with\n> specific bases\" as it claims to do in the comment.\n>\n> Ed Maste writes:\n>> PATTERNS(\"elixir\",\n>> \t \"^[ \\t]*((def(macro|module|impl|protocol|p)?|test)[ \\t].*)$\",\n>> \t /* Atoms, names, and module attributes */\n>> -\t \"|[@:]?[a-zA-Z0-9@_?!]+\"\n>> +\t \"[@:]?[a-zA-Z0-9@_?!]+\"\n>> \t /* Numbers with specific base */\n>> \t \"|[-+]?0[xob][0-9a-fA-F]+\"\n>\n> Here, things like \"+0bad\" would match as a base 2 number, which doesn't\n> seem right. If it's intended to match that broadly, I'd have expected a\n> comment to that effect.\n\nNo need for such a comment, as it is implicit that we assume the\nuser writes reasonable text that our patterns try to match.\n"}]}