{"thread":{"id":"34067","subject":"[PATCH] git-remote-mediawiki: Fix a bug in a regexp","startedAt":"2013-06-08T13:35:10Z","lastAt":"2013-06-09T05:21:43Z","messageCount":6,"participants":["Célestin Matte","Matthieu Moy","Jeff King","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"219821","messageId":"1370698510-11649-1-git-send-email-celestin.matte@ensimag.fr","threadId":"34067","inReplyTo":null,"subject":"[PATCH] git-remote-mediawiki: Fix a bug in a regexp","fromName":"Célestin Matte","fromEmail":"celestin.matte@ensimag.fr","sentAt":"2013-06-08T13:35:10Z","receivedAt":"2013-06-08T13:35:10Z","isPatch":true,"sender":{"key":"celestin.matte@ensimag.fr","avatar":"https://avatars.githubusercontent.com/u/2753554?v=4"},"body":"In Perl, '\\n' is not a newline, but instead a literal backslash followed by an\n\"n\". As the output of \"rev-list --first-parent\" is line-oriented, what we want\nhere is a newline.\n\nSigned-off-by: Célestin Matte \nSigned-off-by: Matthieu Moy \n---\n contrib/mw-to-git/git-remote-mediawiki.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl\nindex 7af202f..a06bc31 100755\n--- a/contrib/mw-to-git/git-remote-mediawiki.perl\n+++ b/contrib/mw-to-git/git-remote-mediawiki.perl\n@@ -1190,7 +1190,7 @@ sub mw_push_revision {\n \t\t# history (linearized with --first-parent)\n \t\tprint STDERR \"Warning: no common ancestor, pushing complete history\\n\";\n \t\tmy $history = run_git(\"rev-list --first-parent --children $local\");\n-\t\tmy @history = split('\\n', $history);\n+\t\tmy @history = split(/\\n/, $history);\n \t\t@history = @history[1..$#history];\n \t\tforeach my $line (reverse @history) {\n \t\t\tmy @commit_info_split = split(/ |\\n/, $line);\n-- \n1.7.9.5\n"},{"id":"219852","messageId":"vpqmwr0v45b.fsf@anie.imag.fr","threadId":"34067","inReplyTo":"1370698510-11649-1-git-send-email-celestin.matte@ensimag.fr","subject":"Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2013-06-08T18:38:56Z","receivedAt":"2013-06-08T18:38:56Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Célestin Matte writes:\n\n> In Perl, '\\n' is not a newline, but instead a literal backslash followed by an\n> \"n\". As the output of \"rev-list --first-parent\" is line-oriented, what we want\n> here is a newline.\n\nThis is right, but the code actually worked the way it was. I'm not\nsure, but my understanding is that '\\n' is the string \"backslash\nfollowed by n\", but interpreted as a regexp, it is a newline.\n\nThe new code looks better than the old one, but the log message may be\nimproved.\n\nIn any case, Acked-by: Matthieu Moy \n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"219859","messageId":"51B38F44.6080304@ensimag.fr","threadId":"34067","inReplyTo":"vpqmwr0v45b.fsf@anie.imag.fr","subject":"Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp","fromName":"Célestin Matte","fromEmail":"celestin.matte@ensimag.fr","sentAt":"2013-06-08T20:08:36Z","receivedAt":"2013-06-08T20:08:36Z","isPatch":true,"sender":{"key":"celestin.matte@ensimag.fr","avatar":"https://avatars.githubusercontent.com/u/2753554?v=4"},"body":"Le 08/06/2013 20:38, Matthieu Moy a écrit :> This is right, but the code\nactually worked the way it was. I'm not\n> sure, but my understanding is that '\\n' is the string \"backslash\n> followed by n\", but interpreted as a regexp, it is a newline.\n>\n> The new code looks better than the old one, but the log message may be\n> improved.\n\nIs this better?\n\n\"\nIn Perl, '\\n' is not a newline, but instead the string composed of a\nbackslash followed by an \"n\". To match newlines, one has to use the /\\n/\nregexp. As the output of \"rev-list --first-parent\" is line-oriented,\nwhat we want here is to match newlines, and not the \"\\n\" string.\n\"\n\n-- \nCélestin Matte\n"},{"id":"219872","messageId":"20130609025708.GB30393@sigill.intra.peff.net","threadId":"34067","inReplyTo":"vpqmwr0v45b.fsf@anie.imag.fr","subject":"Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-06-09T02:57:09Z","receivedAt":"2013-06-09T02:57:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 08, 2013 at 08:38:56PM +0200, Matthieu Moy wrote:\n\n> Célestin Matte writes:\n> \n> > In Perl, '\\n' is not a newline, but instead a literal backslash followed by an\n> > \"n\". As the output of \"rev-list --first-parent\" is line-oriented, what we want\n> > here is a newline.\n> \n> This is right, but the code actually worked the way it was. I'm not\n> sure, but my understanding is that '\\n' is the string \"backslash\n> followed by n\", but interpreted as a regexp, it is a newline.\n\nYes, the relevant doc (from \"perldoc -f split\") is:\n\n The pattern \"/PATTERN/\" may be replaced with an expression to specify\n patterns that vary at runtime. (To do runtime compilation only once,\n use \"/$variable/o\".)\n\nSo it is treating \"\\n\" as an expression and compiling the regex each\ntime through (though I think modern perl may be smart enough to realize\nit is a constant expression and compile the regex only once). You would\nget the same behavior with this:\n\n split $arg, $data;\n\nif $arg contained '\\n'. Of course, you _also_ get the same thing if you\nuse a literal newline (either \"\\n\" or if $arg contained a literal\nnewline), because they function the same in a regex. In other words, it\ndoes not matter which you use because perl's interpolation of \"\\n\" and\nthe regex expansion of \"\\n\" are identical: t hey both mean a newline.\n\nA more subtle example that shows what is going on is this:\n\n split '.', $data;\n\nIf you feed that \"foo.bar.baz\", it does not split it into three words;\neach character is a delimiter, because the dot is compiled to a regex.\n\n> The new code looks better than the old one, but the log message may be\n> improved.\n\nAgreed. I think the best explanation is something like:\n\n Perl's split function takes a regex pattern argument. You can also\n feed it an expression, which is then compiled into a regex at runtime.\n It therefore works to pass your pattern via single quotes, but it is\n much less obvious to a reader that the argument is meant to be a\n regex, not a static string. Using the traditional slash-delimiters\n makes this easier to read.\n\n-Peff\n"},{"id":"219881","messageId":"CAPig+cQ4Juwz0TOGzQjhGRZn8JofiYUG9xT9kJhoj5Z6yAMoGw@mail.gmail.com","threadId":"34067","inReplyTo":"1370698510-11649-1-git-send-email-celestin.matte@ensimag.fr","subject":"Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-06-09T05:11:45Z","receivedAt":"2013-06-09T05:11:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 8, 2013 at 9:35 AM, Célestin Matte\n wrote:\n> In Perl, '\\n' is not a newline, but instead a literal backslash followed by an\n> \"n\". As the output of \"rev-list --first-parent\" is line-oriented, what we want\n> here is a newline.\n>\n> Signed-off-by: Célestin Matte \n> Signed-off-by: Matthieu Moy \n> ---\n> contrib/mw-to-git/git-remote-mediawiki.perl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl\n> index 7af202f..a06bc31 100755\n> --- a/contrib/mw-to-git/git-remote-mediawiki.perl\n> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl\n> @@ -1190,7 +1190,7 @@ sub mw_push_revision {\n> # history (linearized with --first-parent)\n> print STDERR \"Warning: no common ancestor, pushing complete history\\n\";\n> my $history = run_git(\"rev-list --first-parent --children $local\");\n> - my @history = split('\\n', $history);\n> + my @history = split(/\\n/, $history);\n> @history = @history[1..$#history];\n> foreach my $line (reverse @history) {\n> my @commit_info_split = split(/ |\\n/, $line);\n\nIt would be quite acceptable to include this patch in your existing\npatch series.\n"},{"id":"219883","messageId":"CAPig+cSCLsMQ3Xg9UKury41G0vHddnL1PVKBJL_N2amA9e0eyQ@mail.gmail.com","threadId":"34067","inReplyTo":"20130609025708.GB30393@sigill.intra.peff.net","subject":"Re: [PATCH] git-remote-mediawiki: Fix a bug in a regexp","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-06-09T05:21:43Z","receivedAt":"2013-06-09T05:21:43Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 8, 2013 at 10:57 PM, Jeff King wrote:\n> On Sat, Jun 08, 2013 at 08:38:56PM +0200, Matthieu Moy wrote:\n>\n>> Célestin Matte writes:\n>>\n>> > In Perl, '\\n' is not a newline, but instead a literal backslash followed by an\n>> > \"n\". As the output of \"rev-list --first-parent\" is line-oriented, what we want\n>> > here is a newline.\n>>\n>> This is right, but the code actually worked the way it was. I'm not\n>> sure, but my understanding is that '\\n' is the string \"backslash\n>> followed by n\", but interpreted as a regexp, it is a newline.\n>\n> Yes, the relevant doc (from \"perldoc -f split\") is:\n>\n> The pattern \"/PATTERN/\" may be replaced with an expression to specify\n> patterns that vary at runtime. (To do runtime compilation only once,\n> use \"/$variable/o\".)\n>\n> So it is treating \"\\n\" as an expression and compiling the regex each\n> time through ...\n\nI read this as saying only that static /PATTERN/ can also be a\ncomposed /$PATTERN/. It does not indicate how string 'PATTERN' is\ntreated, nor does any other part of \"perldoc -f split\" make special\nmention of string 'PATTERN'. In fact, the only explanation I found\nregarding string 'PATTERN' is in my Camel book (3rd edition, page 796)\nin a parenthesized comment:\n\n (... if you supply a string instead of a regular expression, it'll be\n interpreted as a regular expression anyway.)\n\n>> The new code looks better than the old one, but the log message may be\n>> improved.\n>\n> Agreed. I think the best explanation is something like:\n>\n> Perl's split function takes a regex pattern argument. You can also\n> feed it an expression, which is then compiled into a regex at runtime.\n> It therefore works to pass your pattern via single quotes, but it is\n> much less obvious to a reader that the argument is meant to be a\n> regex, not a static string. Using the traditional slash-delimiters\n> makes this easier to read.\n\nSounds good to me.\n"}]}