{"thread":{"id":"50748","subject":"[GSoC][RFC/PATCH 0/1]","startedAt":"2019-03-15T15:02:11Z","lastAt":"2019-03-16T18:45:23Z","messageCount":5,"participants":["jkapil.cs@gmail.com","Thomas Gummerer"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"371594","messageId":"20190315150142.20672-1-jkapil.cs@gmail.com","threadId":"50748","inReplyTo":null,"subject":"[GSoC][RFC/PATCH 0/1]","fromName":"","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-15T15:01:41Z","receivedAt":"2019-03-15T15:02:11Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"From: Kapil \n\n\nKapil (1):\n userdiff - support shell script\n\n t/t4018-diff-funcname.sh | 1 +\n t/t4034-diff-words.sh | 1 +\n userdiff.c | 5 +++++\n 3 files changed, 7 insertions(+)\n\n-- \n2.14.2\n\n"},{"id":"371595","messageId":"20190315150142.20672-2-jkapil.cs@gmail.com","threadId":"50748","inReplyTo":"20190315150142.20672-1-jkapil.cs@gmail.com","subject":"[GSoC][RFC/PATCH 1/1] userdiff - support shell script","fromName":"","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-15T15:01:42Z","receivedAt":"2019-03-15T15:02:15Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"From: Kapil \n\nMade a minor change of adding regex for identifying functions in shell scripts.\nThis regex has been tested separately from git.\nFor testing with git, please let me know.\n\nThis is just to get familiar with whole workflow, regex for word_regex parameter will be added in next patch.\n\n---\n t/t4018-diff-funcname.sh | 1 +\n t/t4034-diff-words.sh | 1 +\n userdiff.c | 5 +++++\n 3 files changed, 7 insertions(+)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 22f9f88f0a..68092eaeaa 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -44,6 +44,7 @@ diffpatterns=\"\n \tpython\n \truby\n \ttex\n+\tshell\n \tcustom1\n \tcustom2\n \tcustom3\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 912df91226..de21f36d97 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -314,6 +314,7 @@ test_language_driver php\n test_language_driver python\n test_language_driver ruby\n test_language_driver tex\n+test_language_driver shell\n \n test_expect_success 'word-diff with diff.sbe' '\n \tcat >expect <<-\\EOF &&\ndiff --git a/userdiff.c b/userdiff.c\nindex 3a78fbf504..1743608da2 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -170,6 +170,11 @@ IPATTERN(\"css\",\n \t \"-?[_a-zA-Z][-_a-zA-Z0-9]*\" /* identifiers */\n \t \"|-?[0-9]+|\\\\#[0-9a-fA-F]+\" /* numbers */\n ),\n+\n+PATTERNS(\"shell\",\n+\t\"([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\\\([[:space:]]*\\\\)[[:space:]]*\\\\{[[:space:]\\\\(\\\\)|\\\\[<>?`/~!:\\'\\\"%^&*+A-Za-z0-9_$=@;#,.-]*\\\\}\",\n+\t\".*\"\n+),\n { \"default\", NULL, -1, { NULL, 0 } },\n };\n #undef PATTERNS\n-- \n2.14.2\n\n"},{"id":"371633","messageId":"20190315223958.GI16414@hank.intra.tgummerer.com","threadId":"50748","inReplyTo":"20190315150142.20672-1-jkapil.cs@gmail.com","subject":"Re: [GSoC][RFC/PATCH 0/1]","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-03-15T22:39:58Z","receivedAt":"2019-03-15T22:40:03Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/15, jkapil.cs@gmail.com wrote:\n> From: Kapil \n> \n\nThe cover letter should give reviewers a short introduction what a\npatch series is about. It's often not worth including a cover letter\nif there is only a single patch, or only a couple of patches.\n\nIt is okay to send a cover letter for a single patch, however it\nshould include some explanation for the patch series, and not just\nduplicate what's said in the commit message of the actual patch. An\nempty cover letter as this one here is just noise, and should be\nomitted.\n\n> Kapil (1):\n> userdiff - support shell script\n> \n> t/t4018-diff-funcname.sh | 1 +\n> t/t4034-diff-words.sh | 1 +\n> userdiff.c | 5 +++++\n> 3 files changed, 7 insertions(+)\n> \n> -- \n> 2.14.2\n> \n"},{"id":"371634","messageId":"20190315230515.GJ16414@hank.intra.tgummerer.com","threadId":"50748","inReplyTo":"20190315150142.20672-2-jkapil.cs@gmail.com","subject":"Re: [GSoC][RFC/PATCH 1/1] userdiff - support shell script","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-03-15T23:05:15Z","receivedAt":"2019-03-15T23:05:21Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"> Subject: [GSoC][RFC/PATCH 1/1] userdiff - support shell script\n\nWe usually have the subject in the format \": \", so in this case it could be \"userdiff: support shell\nscript\" (without the quotes of course).\n\nOn 03/15, jkapil.cs@gmail.com wrote:\n> From: Kapil \n\nIdeally the name you are using for your contribution should be your\nfull name, including your last name.\n\n> Made a minor change of adding regex for identifying functions in shell scripts.\n\nThe commit message should be in the imperative mood, see also\nDocumentation/SubmittingPatches. In addition the \"minor change\" bit\nshould be omitted, as it doesn't add any actual value to the commit\nmessage.\n\nSo the above should just be \"Add regex for identifying functions in\nshell scripts\".\n\n> This regex has been tested separately from git.\n> For testing with git, please let me know.\n\nFor testing you could have a look how other userdiff drivers are\ntested in t4034. The easiest way is probably to have a look at a\npatch for a userdiff pattern that was added previously. For example\nhave a look at 0719f3eecd (\"userdiff: add built-in pattern for CSS\",\n2016-06-03). (I found this commit by looking at the git blame output\nfor t4034.)\n\nBoth the two lines above and the line below should go below the ---\nline, as they should not be part of the history in the git project,\nbut serve only as additional commentary for reviewers looking at this\npatch.\n\n> This is just to get familiar with whole workflow, regex for word_regex parameter will be added in next patch.\n\nThere should be a Signed-off-by: line here. For its significance\nplease see Documentation/SubmittingPatches.\n\nAnother thing you should consider is whether someone attempted to add\nsupport for this before. And indeed a quick search through\npublic-inbox turns up [*1*]. It's always worth having a look at\nprevious work, and things that can be learned from that.\n\n*1*: https://public-inbox.org/git/20170329165331.17742-1-pickfire@riseup.net/#t\n\n> ---\n> t/t4018-diff-funcname.sh | 1 +\n> t/t4034-diff-words.sh | 1 +\n> userdiff.c | 5 +++++\n> 3 files changed, 7 insertions(+)\n> \n> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\n> index 22f9f88f0a..68092eaeaa 100755\n> --- a/t/t4018-diff-funcname.sh\n> +++ b/t/t4018-diff-funcname.sh\n> @@ -44,6 +44,7 @@ diffpatterns=\"\n> \tpython\n> \truby\n> \ttex\n> +\tshell\n> \tcustom1\n> \tcustom2\n> \tcustom3\n> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\n> index 912df91226..de21f36d97 100755\n> --- a/t/t4034-diff-words.sh\n> +++ b/t/t4034-diff-words.sh\n> @@ -314,6 +314,7 @@ test_language_driver php\n> test_language_driver python\n> test_language_driver ruby\n> test_language_driver tex\n> +test_language_driver shell\n> \n> test_expect_success 'word-diff with diff.sbe' '\n> \tcat >expect <<-\\EOF &&\n> diff --git a/userdiff.c b/userdiff.c\n> index 3a78fbf504..1743608da2 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -170,6 +170,11 @@ IPATTERN(\"css\",\n> \t \"-?[_a-zA-Z][-_a-zA-Z0-9]*\" /* identifiers */\n> \t \"|-?[0-9]+|\\\\#[0-9a-fA-F]+\" /* numbers */\n> ),\n> +\n> +PATTERNS(\"shell\",\n> +\t\"([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\\\([[:space:]]*\\\\)[[:space:]]*\\\\{[[:space:]\\\\(\\\\)|\\\\[<>?`/~!:\\'\\\"%^&*+A-Za-z0-9_$=@;#,.-]*\\\\}\",\n> +\t\".*\"\n> +),\n> { \"default\", NULL, -1, { NULL, 0 } },\n> };\n> #undef PATTERNS\n> -- \n> 2.14.2\n> \n"},{"id":"371657","messageId":"20190316184516.GL16414@hank.intra.tgummerer.com","threadId":"50748","inReplyTo":"CAMknYEP3dp0a9B4ZW5F8kt+rrRZagWaVYBMhYRWj68UgYrF-GA@mail.gmail.com","subject":"Re: [GSoC][RFC/PATCH 1/1] userdiff - support shell script","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-03-16T18:45:16Z","receivedAt":"2019-03-16T18:45:23Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"[please keep everyone in Cc, especially the mailing list if you are\nresponding to review comments. I added the Cc's back]\n\nOn 03/16, Kapil Jain wrote:\n> On Sat, Mar 16, 2019 at 4:35 AM Thomas Gummerer wrote:\n> >\n> > For testing you could have a look how other userdiff drivers are\n> > tested in t4034. The easiest way is probably to have a look at a\n> > patch for a userdiff pattern that was added previously. For example\n> > have a look at 0719f3eecd (\"userdiff: add built-in pattern for CSS\",\n> > 2016-06-03). (I found this commit by looking at the git blame output\n> > for t4034.)\n> > *1*: https://public-inbox.org/git/20170329165331.17742-1-pickfire@riseup.net/#t\n> \n> Thanks, this helped in understanding testing process.\n> \n> > Both the two lines above and the line below should go below the ---\n> > line, as they should not be part of the history in the git project,\n> > but serve only as additional commentary for reviewers looking at this\n> > patch.\n> \n> I looked at other patches like\n> (https://public-inbox.org/git/CAC05386q2iGoiJ_fRgwoOTF23exEN2D1+oh4VjajEvYQ58O1TQ@mail.gmail.com/)\n> The patch has the email body above --- line. did i missed to\n> understanding something here ?\n\nRight, what I was trying but failed to explain is that everything\nabove the --- line is part of the commit message, that gets recorded\nin the history of the project.\n\nHowever the lines I referred to, should not go in the commit message,\nas they are just commentary that helps reviewers, but don't actually\nexplain the patch itself. That additional commentary should go below\nthe --- line, similar to the\n\n Notes:\n Travis CI results: https://travis-ci.org/nbelakovski/git/builds/432320949\n\nlines in the email you mentioned.\n"}]}