| Message ID | 20190322160615.27124-1-jkapil.cs@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GSoC,RFC/PATCH] userdiff: added support for diffing shell scripts | expand |
On Fri, Mar 22, 2019 at 5:08 PM Kapil Jain <jkapil.cs@gmail.com> wrote: > > Signed-off-by: Kapil Jain <jkapil.cs@gmail.com> > --- > > The test written does not pass, imo there's some problem with the regex part. > please let me know about the fault. To save some work by people who could help you, it might be a good idea to show the output of the failing test, for example the output of `./t4034-diff-words.sh -i -v` or `./t4034-diff-words.sh -i -v -x`.
On Sun, Mar 24, 2019 at 1:34 AM Christian Couder <christian.couder@gmail.com> wrote: > > To save some work by people who could help you, it might be a good > idea to show the output of the failing test, for example the output of > `./t4034-diff-words.sh -i -v` or `./t4034-diff-words.sh -i -v -x`. Looks like i just needed to know about -i, -v and -x switches, they helped in debugging. The problem was with the expect file. It is resolved now. Thanks, will be resubmitting with updated expect file. is the parser used to parse the expect file specific to git ? or is it some general one ?
On Sun, Mar 24, 2019 at 9:04 AM Kapil Jain <jkapil.cs@gmail.com> wrote: > > On Sun, Mar 24, 2019 at 1:34 AM Christian Couder > <christian.couder@gmail.com> wrote: > > > > To save some work by people who could help you, it might be a good > > idea to show the output of the failing test, for example the output of > > `./t4034-diff-words.sh -i -v` or `./t4034-diff-words.sh -i -v -x`. > > Looks like i just needed to know about -i, -v and -x switches, they > helped in debugging. > The problem was with the expect file. It is resolved now. Great! > Thanks, will be resubmitting with updated expect file. It looks like you sent "[GSoC][RFC/PATCH 2/2] userdiff: added shell script support, clears test". I think though that it should have been named "[GSoC][RFC/PATCH v2] userdiff: added shell script support, clears test". "2/2" means that it is patch number 2 in a patch series that has 2 patches (which should be marked with "1/2" and "2/2"). When resubmitting an already submitted patch (or patch series) we ask for a version number like "v2", "v3", so that we can know that it has already been submitted. `git format-patch -v 2 ...` will automatically add "v2". > is the parser used to parse the expect file specific to git ? or is it > some general one ? The test_language_driver() function used to test the regexps is defined in t4034-diff-words.sh and it calls the word_diff() function (also defined in t4034-diff-words.sh) which is: word_diff () { test_must_fail git diff --no-index "$@" pre post >output && test_decode_color <output >output.decrypted && test_cmp expect output.decrypted } So it uses test_decode_color() and test_cmp() that are defined in t/test-lib-functions.sh. test_cmp() in turn is defined using GIT_TEST_CMP which is usually either "diff -u" or "diff -c".
On Sun, Mar 24, 2019 at 2:49 PM Christian Couder <christian.couder@gmail.com> wrote: > > The test_language_driver() function used to test the regexps is > ... > GIT_TEST_CMP which is usually either "diff -u" or "diff -c". Thanks. please provide some insights on the regex mentioned below: + +PATTERNS("shell", + /* Function Names */ + "([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\([[:space:]]*\\)[[:space:]]*\\{", + /* Words */ + "([$#@A-Za-z_\"\'][$#@A-Za-z0-9_\"\']*)"), + reference mail: https://public-inbox.org/git/20190324084523.8744-1-jkapil.cs@gmail.com/. please let me know if the regex is not self explanatory.
On 03/24, Kapil Jain wrote: > On Sun, Mar 24, 2019 at 2:49 PM Christian Couder > <christian.couder@gmail.com> wrote: > > > > The test_language_driver() function used to test the regexps is > > ... > > GIT_TEST_CMP which is usually either "diff -u" or "diff -c". > > Thanks. > > please provide some insights on the regex mentioned below: I had previously mentioned that this project was attempted already in my email at [*1*]. Did you take a look at the thread I linked to there, and the regex used? I still feel like that previous experience is something you could learn from. But that said, I think your assumption in the other email that the output should be [-$TEST_DIRECTORY-] {+$TEST_DIR+} might not be correct. The tests are using 'git diff --word-diff=color', rather than 'git diff --word-diff=plain'. Only the latter would add the [- -] and {+ +} around the changed words, while the former adds the color, which the tests are testing for. *1*: https://public-inbox.org/git/20190315230515.GJ16414@hank.intra.tgummerer.com/ > + > +PATTERNS("shell", > + /* Function Names */ > + "([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\([[:space:]]*\\)[[:space:]]*\\{", > + /* Words */ > + "([$#@A-Za-z_\"\'][$#@A-Za-z0-9_\"\']*)"), > + > > reference mail: > https://public-inbox.org/git/20190324084523.8744-1-jkapil.cs@gmail.com/. > please let me know if the regex is not self explanatory.
On Fri, Mar 29, 2019 at 3:00 AM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > > I had previously mentioned that this project was attempted already in > my email at [*1*]. Did you take a look at the thread I linked to > there, and the regex used? I still feel like that previous experience > is something you could learn from. I saw it, and the regex i have used is a little different from that one. > But that said, I think your assumption in the other email that the > output should be > > [-$TEST_DIRECTORY-] > {+$TEST_DIR+} > > might not be correct. The tests are using 'git diff > --word-diff=color', rather than 'git diff --word-diff=plain'. Only > the latter would add the [- -] and {+ +} around the changed words, > while the former adds the color, which the tests are testing for. This makes sense, i will write more tests for this.
On 03/29, Kapil Jain wrote: > On Fri, Mar 29, 2019 at 3:00 AM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > > > > > I had previously mentioned that this project was attempted already in > > my email at [*1*]. Did you take a look at the thread I linked to > > there, and the regex used? I still feel like that previous experience > > is something you could learn from. > > I saw it, and the regex i have used is a little different from that one. Right, so one thing you should definitely do is to compare the regex there and the regex you have, and understand why there are differences when they are trying to do the same thing. Using that knowledge you should be able to improve the regex in your patch as well.
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh index 912df91226..74366e6826 100755 --- a/t/t4034-diff-words.sh +++ b/t/t4034-diff-words.sh @@ -314,6 +314,8 @@ test_language_driver php test_language_driver python test_language_driver ruby test_language_driver tex +test_language_driver shell + test_expect_success 'word-diff with diff.sbe' ' cat >expect <<-\EOF && diff --git a/t/t4034/shell/expect b/t/t4034/shell/expect new file mode 100644 index 0000000000..f2f65e7a9b --- /dev/null +++ b/t/t4034/shell/expect @@ -0,0 +1,6 @@ +<BOLD>diff --git a/pre b/post<RESET> +<BOLD>index 2fc00ad..cd34305 100644<RESET> +<BOLD>--- a/pre<RESET> +<BOLD>+++ b/post<RESET> +<CYAN>@@ -1 +1 @@<RESET> +<RED>[-$TEST_DIRECTORY-]<RESET><GREEN>{+$TEST_DIR+}<RESET> diff --git a/t/t4034/shell/post b/t/t4034/shell/post new file mode 100644 index 0000000000..43a84e0188 --- /dev/null +++ b/t/t4034/shell/post @@ -0,0 +1 @@ +$TEST_DIR diff --git a/t/t4034/shell/pre b/t/t4034/shell/pre new file mode 100644 index 0000000000..32440f90b7 --- /dev/null +++ b/t/t4034/shell/pre @@ -0,0 +1 @@ +$TEST_DIRECTORY diff --git a/userdiff.c b/userdiff.c index 3a78fbf504..936447a0bc 100644 --- a/userdiff.c +++ b/userdiff.c @@ -158,6 +158,13 @@ PATTERNS("csharp", "[a-zA-Z_][a-zA-Z0-9_]*" "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?" "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"), + +PATTERNS("shell", + /* Function Names */ + "([A-Za-z_][A-Za-z0-9_]*)[[:space:]]*\\([[:space:]]*\\)[[:space:]]*\\{", + /* Words */ + "([$#@A-Za-z_\"\'][$#@A-Za-z0-9_\"\']*)"), + IPATTERN("css", "![:;][[:space:]]*$\n" "^[_a-z0-9].*$",
Signed-off-by: Kapil Jain <jkapil.cs@gmail.com> --- The test written does not pass, imo there's some problem with the regex part. please let me know about the fault. t/t4034-diff-words.sh | 2 ++ t/t4034/shell/expect | 6 ++++++ t/t4034/shell/post | 1 + t/t4034/shell/pre | 1 + userdiff.c | 7 +++++++ 5 files changed, 17 insertions(+) create mode 100644 t/t4034/shell/expect create mode 100644 t/t4034/shell/post create mode 100644 t/t4034/shell/pre