{"thread":{"id":"50805","subject":"[GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","startedAt":"2019-03-22T16:06:35Z","lastAt":"2019-03-29T19:07:46Z","messageCount":9,"participants":["Kapil Jain","Christian Couder","Thomas Gummerer"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"372256","messageId":"20190322160615.27124-1-jkapil.cs@gmail.com","threadId":"50805","inReplyTo":null,"subject":"[GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Kapil Jain","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-22T16:06:15Z","receivedAt":"2019-03-22T16:06:35Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"Signed-off-by: Kapil Jain \n---\n\nThe test written does not pass, imo there's some problem with the regex part.\nplease let me know about the fault.\n\n t/t4034-diff-words.sh | 2 ++\n t/t4034/shell/expect | 6 ++++++\n t/t4034/shell/post | 1 +\n t/t4034/shell/pre | 1 +\n userdiff.c | 7 +++++++\n 5 files changed, 17 insertions(+)\n create mode 100644 t/t4034/shell/expect\n create mode 100644 t/t4034/shell/post\n create mode 100644 t/t4034/shell/pre\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 912df91226..74366e6826 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -314,6 +314,8 @@ 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 \n test_expect_success 'word-diff with diff.sbe' '\n \tcat >expect <<-\\EOF &&\ndiff --git a/t/t4034/shell/expect b/t/t4034/shell/expect\nnew file mode 100644\nindex 0000000000..f2f65e7a9b\n--- /dev/null\n+++ b/t/t4034/shell/expect\n@@ -0,0 +1,6 @@\n+diff --git a/pre b/post\n+index 2fc00ad..cd34305 100644\n+--- a/pre\n++++ b/post\n+@@ -1 +1 @@\n+[-$TEST_DIRECTORY-]{+$TEST_DIR+}\ndiff --git a/t/t4034/shell/post b/t/t4034/shell/post\nnew file mode 100644\nindex 0000000000..43a84e0188\n--- /dev/null\n+++ b/t/t4034/shell/post\n@@ -0,0 +1 @@\n+$TEST_DIR\ndiff --git a/t/t4034/shell/pre b/t/t4034/shell/pre\nnew file mode 100644\nindex 0000000000..32440f90b7\n--- /dev/null\n+++ b/t/t4034/shell/pre\n@@ -0,0 +1 @@\n+$TEST_DIRECTORY\ndiff --git a/userdiff.c b/userdiff.c\nindex 3a78fbf504..936447a0bc 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -158,6 +158,13 @@ PATTERNS(\"csharp\",\n \t \"[a-zA-Z_][a-zA-Z0-9_]*\"\n \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?\"\n \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\"),\n+\n+PATTERNS(\"shell\",\n+ /* Function Names */\n+ \"([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\\\([[:space:]]*\\\\)[[:space:]]*\\\\{\",\n+ /* Words */\n+ \"([$#@A-Za-z_\\\"\\'][$#@A-Za-z0-9_\\\"\\']*)\"),\n+\n IPATTERN(\"css\",\n \t \"![:;][[:space:]]*$\\n\"\n \t \"^[_a-z0-9].*$\",\n-- \n2.14.2\n\n"},{"id":"372313","messageId":"CAP8UFD3K4ft7UVSFeaQzKVVGFPwcLcpTKB+sqFN9X9_j_A093w@mail.gmail.com","threadId":"50805","inReplyTo":"20190322160615.27124-1-jkapil.cs@gmail.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-03-23T20:04:09Z","receivedAt":"2019-03-23T20:04:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Mar 22, 2019 at 5:08 PM Kapil Jain wrote:\n>\n> Signed-off-by: Kapil Jain \n> ---\n>\n> The test written does not pass, imo there's some problem with the regex part.\n> please let me know about the fault.\n\nTo save some work by people who could help you, it might be a good\nidea to show the output of the failing test, for example the output of\n`./t4034-diff-words.sh -i -v` or `./t4034-diff-words.sh -i -v -x`.\n"},{"id":"372324","messageId":"CAMknYEMBYxp1QJ3Ds9dmuS4ccHKtExx33d_Kv63UOwaUMm5oUQ@mail.gmail.com","threadId":"50805","inReplyTo":"CAP8UFD3K4ft7UVSFeaQzKVVGFPwcLcpTKB+sqFN9X9_j_A093w@mail.gmail.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Kapil Jain","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-24T08:04:37Z","receivedAt":"2019-03-24T08:04:51Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"On Sun, Mar 24, 2019 at 1:34 AM Christian Couder\n wrote:\n>\n> To save some work by people who could help you, it might be a good\n> idea to show the output of the failing test, for example the output of\n> `./t4034-diff-words.sh -i -v` or `./t4034-diff-words.sh -i -v -x`.\n\nLooks like i just needed to know about -i, -v and -x switches, they\nhelped in debugging.\nThe problem was with the expect file. It is resolved now.\n\nThanks, will be resubmitting with updated expect file.\n\nis the parser used to parse the expect file specific to git ? or is it\nsome general one ?\n"},{"id":"372346","messageId":"20190324084523.8744-1-jkapil.cs@gmail.com","threadId":"50805","inReplyTo":"CAP8UFD3K4ft7UVSFeaQzKVVGFPwcLcpTKB+sqFN9X9_j_A093w@mail.gmail.com","subject":"[GSoC][RFC/PATCH 2/2] userdiff: added shell script support, clears test","fromName":"Kapil Jain","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-24T08:45:23Z","receivedAt":"2019-03-24T08:45:50Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"Signed-off-by: Kapil Jain \n---\n\nThe test passes now, but imo the regex is not working,\nbecause the output of git diff with shell regex remains same\nas earlier it was without shell regex.\n\nwithout shell regex the output was shown as:\n-$TEST_DIRECTORY\n+$TEST_DIR\n\nwith shell regex the output should be:\n[-$TEST_DIRECTORY-]\n{+$TEST_DIR+}\n\nbut even with shell regex the output is:\n-$TEST_DIRECTORY\n+$TEST_DIR\n\nsome thoughts on regex would be helpful.\nthe shell regex is below:\n\n+\n+PATTERNS(\"shell\",\n+ /* Function Names */\n+ \"([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\\\([[:space:]]*\\\\)[[:space:]]*\\\\{\",\n+ /* Words */\n+ \"([$#@A-Za-z_\\\"\\'][$#@A-Za-z0-9_\\\"\\']*)\"),\n+\n\nThanks.\n\n t/t4034/shell/expect | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4034/shell/expect b/t/t4034/shell/expect\nindex f2f65e7a9b..1f0d1e1d43 100644\n--- a/t/t4034/shell/expect\n+++ b/t/t4034/shell/expect\n@@ -1,6 +1,6 @@\n diff --git a/pre b/post\n-index 2fc00ad..cd34305 100644\n+index 32440f9..43a84e0 100644\n --- a/pre\n +++ b/post\n @@ -1 +1 @@\n-[-$TEST_DIRECTORY-]{+$TEST_DIR+}\n+$TEST_DIRECTORY$TEST_DIR\n-- \n2.20.1\n\n"},{"id":"372347","messageId":"CAP8UFD0cWSTZPqqVwTFyYL06S+6aT_EiLPW6jUE0AH9puxevmg@mail.gmail.com","threadId":"50805","inReplyTo":"CAMknYEMBYxp1QJ3Ds9dmuS4ccHKtExx33d_Kv63UOwaUMm5oUQ@mail.gmail.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-03-24T09:19:12Z","receivedAt":"2019-03-24T09:19:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Mar 24, 2019 at 9:04 AM Kapil Jain wrote:\n>\n> On Sun, Mar 24, 2019 at 1:34 AM Christian Couder\n> wrote:\n> >\n> > To save some work by people who could help you, it might be a good\n> > idea to show the output of the failing test, for example the output of\n> > `./t4034-diff-words.sh -i -v` or `./t4034-diff-words.sh -i -v -x`.\n>\n> Looks like i just needed to know about -i, -v and -x switches, they\n> helped in debugging.\n> The problem was with the expect file. It is resolved now.\n\nGreat!\n\n> Thanks, will be resubmitting with updated expect file.\n\nIt looks like you sent \"[GSoC][RFC/PATCH 2/2] userdiff: added shell\nscript support, clears test\". I think though that it should have been\nnamed \"[GSoC][RFC/PATCH v2] userdiff: added shell script support,\nclears test\".\n\n\"2/2\" means that it is patch number 2 in a patch series that has 2\npatches (which should be marked with \"1/2\" and \"2/2\"). When\nresubmitting an already submitted patch (or patch series) we ask for a\nversion number like \"v2\", \"v3\", so that we can know that it has\nalready been submitted.\n\n`git format-patch -v 2 ...` will automatically add \"v2\".\n\n> is the parser used to parse the expect file specific to git ? or is it\n> some general one ?\n\nThe test_language_driver() function used to test the regexps is\ndefined in t4034-diff-words.sh and it calls the word_diff() function\n(also defined in t4034-diff-words.sh) which is:\n\nword_diff () {\n test_must_fail git diff --no-index \"$@\" pre post >output &&\n test_decode_color output.decrypted &&\n test_cmp expect output.decrypted\n}\n\nSo it uses test_decode_color() and test_cmp() that are defined in\nt/test-lib-functions.sh. test_cmp() in turn is defined using\nGIT_TEST_CMP which is usually either \"diff -u\" or \"diff -c\".\n"},{"id":"372349","messageId":"CAMknYENJ+U4urtSEtwDSLpdwGe=x=uq_HdSs-cT9Z+PT5ZQiLg@mail.gmail.com","threadId":"50805","inReplyTo":"CAP8UFD0cWSTZPqqVwTFyYL06S+6aT_EiLPW6jUE0AH9puxevmg@mail.gmail.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Kapil Jain","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-24T10:04:44Z","receivedAt":"2019-03-24T10:04:58Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"On Sun, Mar 24, 2019 at 2:49 PM Christian Couder\n wrote:\n>\n> The test_language_driver() function used to test the regexps is\n> ...\n> GIT_TEST_CMP which is usually either \"diff -u\" or \"diff -c\".\n\nThanks.\n\nplease provide some insights on the regex mentioned below:\n\n+\n+PATTERNS(\"shell\",\n+ /* Function Names */\n+ \"([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\\\([[:space:]]*\\\\)[[:space:]]*\\\\{\",\n+ /* Words */\n+ \"([$#@A-Za-z_\\\"\\'][$#@A-Za-z0-9_\\\"\\']*)\"),\n+\n\nreference mail:\nhttps://public-inbox.org/git/20190324084523.8744-1-jkapil.cs@gmail.com/.\nplease let me know if the regex is not self explanatory.\n"},{"id":"372649","messageId":"20190328213010.GI32487@hank.intra.tgummerer.com","threadId":"50805","inReplyTo":"CAMknYENJ+U4urtSEtwDSLpdwGe=x=uq_HdSs-cT9Z+PT5ZQiLg@mail.gmail.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-03-28T21:30:10Z","receivedAt":"2019-03-28T21:30:15Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/24, Kapil Jain wrote:\n> On Sun, Mar 24, 2019 at 2:49 PM Christian Couder\n> wrote:\n> >\n> > The test_language_driver() function used to test the regexps is\n> > ...\n> > GIT_TEST_CMP which is usually either \"diff -u\" or \"diff -c\".\n> \n> Thanks.\n> \n> please provide some insights on the regex mentioned below:\n\nI had previously mentioned that this project was attempted already in\nmy email at [*1*]. Did you take a look at the thread I linked to\nthere, and the regex used? I still feel like that previous experience\nis something you could learn from.\n\nBut that said, I think your assumption in the other email that the\noutput should be\n\n[-$TEST_DIRECTORY-]\n{+$TEST_DIR+}\n\nmight not be correct. The tests are using 'git diff\n--word-diff=color', rather than 'git diff --word-diff=plain'. Only\nthe latter would add the [- -] and {+ +} around the changed words,\nwhile the former adds the color, which the tests are testing for.\n\n*1*: https://public-inbox.org/git/20190315230515.GJ16414@hank.intra.tgummerer.com/\n\n> +\n> +PATTERNS(\"shell\",\n> + /* Function Names */\n> + \"([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\\\([[:space:]]*\\\\)[[:space:]]*\\\\{\",\n> + /* Words */\n> + \"([$#@A-Za-z_\\\"\\'][$#@A-Za-z0-9_\\\"\\']*)\"),\n> +\n> \n> reference mail:\n> https://public-inbox.org/git/20190324084523.8744-1-jkapil.cs@gmail.com/.\n> please let me know if the regex is not self explanatory.\n"},{"id":"372687","messageId":"CAMknYEM3Tff7Z5HcdNT9-RDTf53RHi1_scpO8JVo3pAD3-0xSw@mail.gmail.com","threadId":"50805","inReplyTo":"20190328213010.GI32487@hank.intra.tgummerer.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Kapil Jain","fromEmail":"jkapil.cs@gmail.com","sentAt":"2019-03-29T12:13:42Z","receivedAt":"2019-03-29T12:13:56Z","isPatch":true,"sender":{"key":"jkapil.cs@gmail.com","avatar":null},"body":"On Fri, Mar 29, 2019 at 3:00 AM Thomas Gummerer wrote:\n>\n>\n> I had previously mentioned that this project was attempted already in\n> my email at [*1*]. Did you take a look at the thread I linked to\n> there, and the regex used? I still feel like that previous experience\n> is something you could learn from.\n\nI saw it, and the regex i have used is a little different from that one.\n\n> But that said, I think your assumption in the other email that the\n> output should be\n>\n> [-$TEST_DIRECTORY-]\n> {+$TEST_DIR+}\n>\n> might not be correct. The tests are using 'git diff\n> --word-diff=color', rather than 'git diff --word-diff=plain'. Only\n> the latter would add the [- -] and {+ +} around the changed words,\n> while the former adds the color, which the tests are testing for.\n\nThis makes sense, i will write more tests for this.\n"},{"id":"372725","messageId":"20190329190741.GM32487@hank.intra.tgummerer.com","threadId":"50805","inReplyTo":"CAMknYEM3Tff7Z5HcdNT9-RDTf53RHi1_scpO8JVo3pAD3-0xSw@mail.gmail.com","subject":"Re: [GSoC][RFC/PATCH] userdiff: added support for diffing shell scripts","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-03-29T19:07:41Z","receivedAt":"2019-03-29T19:07:46Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 03/29, Kapil Jain wrote:\n> On Fri, Mar 29, 2019 at 3:00 AM Thomas Gummerer wrote:\n> >\n> >\n> > I had previously mentioned that this project was attempted already in\n> > my email at [*1*]. Did you take a look at the thread I linked to\n> > there, and the regex used? I still feel like that previous experience\n> > is something you could learn from.\n> \n> I saw it, and the regex i have used is a little different from that one.\n\nRight, so one thing you should definitely do is to compare the regex\nthere and the regex you have, and understand why there are differences\nwhen they are trying to do the same thing. Using that knowledge you\nshould be able to improve the regex in your patch as well.\n"}]}