From: Johannes Sixt <j6t@kdbg.org>
To: Jaydeep P Das <jaydeepjd.8914@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] userdiff: add builtin diff driver for Kotlin language.
Date: Tue, 1 Mar 2022 20:47:42 +0100 [thread overview]
Message-ID: <5df2c9ce-b243-0173-befb-e13a6a74e387@kdbg.org> (raw)
In-Reply-To: <20220301155431.2534136-1-jaydeepjd.8914@gmail.com>
Am 01.03.22 um 16:54 schrieb Jaydeep P Das:
> diff --git a/t/t4018/kotlin-nested-fun b/t/t4018/kotlin-nested-fun
> new file mode 100644
> index 0000000000..12186858cb
> --- /dev/null
> +++ b/t/t4018/kotlin-nested-fun
> @@ -0,0 +1,9 @@
> +class LEFT{
> + class CENTER{
> + fun RIGHT( a:Int){
> + //comment
> + //comment
> + ChangeMe
> + }
> + }
> +}
Nice move to include a test with an indented key phrase. The t4018 test
cases all look fine. I don't speek Kotlin, though, so...
> diff --git a/t/t4034/kotlin/expect b/t/t4034/kotlin/expect
> new file mode 100644
> index 0000000000..80eea3e386
> --- /dev/null
> +++ b/t/t4034/kotlin/expect
> @@ -0,0 +1,33 @@
> +<BOLD>diff --git a/pre b/post<RESET>
> +<BOLD>index e8a199a..e6ebebb 100644<RESET>
> +<BOLD>--- a/pre<RESET>
> +<BOLD>+++ b/post<RESET>
> +<CYAN>@@ -1,16 +1,16 @@<RESET>
> +println("Hello World<RED>!\n<RESET><GREEN>?<RESET>")
> +<GREEN>(<RESET>1<GREEN>) (<RESET>-1e10<RED>0xabcdef<RESET><GREEN>) (0xaybcdef)<RESET> '<RED>x<RESET><GREEN>y<RESET>'
> +[<RED>a<RESET><GREEN>x<RESET>] <RED>a<RESET><GREEN>x<RESET>-><RED>b a<RESET><GREEN>y x<RESET>.<RED>b<RESET><GREEN>y<RESET>
> +!<RED>a a<RESET><GREEN>x x<RESET>.inv() <RED>a<RESET><GREEN>x<RESET>*<RED>b a<RESET><GREEN>y x<RESET>&<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>*<RED>b a<RESET><GREEN>y x<RESET>/<RED>b a<RESET><GREEN>y x<RESET>%<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>+<RED>b a<RESET><GREEN>y x<RESET>-<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET> shl <RED>b a<RESET><GREEN>y x<RESET> shr <RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET><<RED>b a<RESET><GREEN>y x<RESET><=<RED>b a<RESET><GREEN>y x<RESET>><RED>b a<RESET><GREEN>y x<RESET>>=<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>==<RED>b a<RESET><GREEN>y x<RESET>!=<RED>b a<RESET><GREEN>y x<RESET>===<RED>b<RESET>
> +<RED>a and b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x xnd y<RESET>
> +<GREEN>x<RESET>^<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET> or <RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>&&<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>||<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>=<RED>b a<RESET><GREEN>y x<RESET>+=<RED>b a<RESET><GREEN>y x<RESET>-=<RED>b a<RESET><GREEN>y x<RESET>*=<RED>b a<RESET><GREEN>y x<RESET>/=<RED>b a<RESET><GREEN>y x<RESET>%=<RED>b a<RESET><GREEN>y x<RESET><<=<RED>b a<RESET><GREEN>y x<RESET>>>=<RED>b a<RESET><GREEN>y x<RESET>&=<RED>b a<RESET><GREEN>y x<RESET>^=<RED>b a<RESET><GREEN>y x<RESET>|=<RED>b<RESET>
> +<RED>a<RESET><GREEN>y<RESET>
> +<GREEN>x<RESET>,y
> diff --git a/t/t4034/kotlin/post b/t/t4034/kotlin/post
> new file mode 100644
> index 0000000000..e6ebebb5e9
> --- /dev/null
> +++ b/t/t4034/kotlin/post
> @@ -0,0 +1,16 @@
> +println("Hello World?")
> +(1) (-1e10) (0xaybcdef) 'y'
> +[x] x->y x.y
> +!x x.inv() x*y x&y
> +x*y x/y x%y
> +x+y x-y
> +x shl y x shr y
> +x<y x<=y x>y x>=y
> +x==y x!=y x===y
> +x xnd y
> +x^y
> +x or y
> +x&&y
> +x||y
> +x=y x+=y x-=y x*=y x/=y x%=y x<<=y x>>=y x&=y x^=y x|=y
> +x,y
> diff --git a/t/t4034/kotlin/pre b/t/t4034/kotlin/pre
> new file mode 100644
> index 0000000000..e8a199adb0
> --- /dev/null
> +++ b/t/t4034/kotlin/pre
> @@ -0,0 +1,16 @@
> +println("Hello World!\n")
> +1 -1e10 0xabcdef 'x'
> +[a] a->b a.b
> +!a a.inv() a*b a&b
> +a*b a/b a%b
> +a+b a-b
> +a shl b a shr b
> +a<b a<=b a>b a>=b
> +a==b a!=b a===b
> +a and b
> +a^b
> +a or b
> +a&&b
> +a||b
> +a=b a+=b a-=b a*=b a/=b a%=b a<<=b a>>=b a&=b a^=b a|=b
> +a,y
I know you just copied an existing test case. But actually, it misses
the important parts of the word regex patterns. In particular, it only
tests that a change of a to x is found, but does not test that the
operators are not split into individual characters. Please have a look
at my series 1cf93847c1ed~..386076ec92c7 and in particular 1cf93847c1ed
to see what you actually want to test. For example, you could test a
change from a+=b to a-=b, i.e., that operators += and -= are not split
into +, -, and =.
> diff --git a/userdiff.c b/userdiff.c
> index 8578cb0d12..f23f098f19 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -168,6 +168,13 @@ PATTERNS("java",
> "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
> "|[-+*/<>%&^|=!]="
> "|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
> +PATTERNS("kotlin",
> + "^[ \t]*(([a-z]+[ \t]+)*(fun|class|interface)[ \t]+.*[ \t]*)$",
I would guess that the trailing [ \t]* is pointless and always empty,
because it is covered by the preceding .*, so you can remove it.
> + /* -- */
> + "[a-zA-Z_][a-zA-Z0-9_]*"
> + "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
The first part intends to match integers and floatingpoint numbers. Word
regex can be loose. This one, however, is too loose. For example, it
treats -e+2 as a single token, but that is actually a whole expression
consisting of several tokens and is not unlikely to occur in real code.
See also 350b87cd6585.
I am pretty sure that, e.g., -1 and +2.5 are both two tokens each, i.e.,
the sign is not part of the number token.
Also, it looks like 3.0e5 is a floating point number; is 3.0E5 not?
> + "|[-+*/<>%&^|=!]="
> + "|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
> PATTERNS("markdown",
> "^ {0,3}#{1,6}[ \t].*",
> /* -- */
-- Hannes
next prev parent reply other threads:[~2022-03-01 19:49 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-01 7:02 [GSoC][PATCH] userdiff: Add diff driver for Kotlin lang and tests Jaydeep P Das
2022-03-01 7:02 ` [PATCH] " Jaydeep P Das
2022-03-01 9:32 ` Junio C Hamano
2022-03-01 9:37 ` Ævar Arnfjörð Bjarmason
2022-03-01 10:27 ` jaydeepjd.8914
2022-03-01 15:54 ` [PATCH] userdiff: add builtin diff driver for Kotlin language Jaydeep P Das
2022-03-01 17:17 ` Junio C Hamano
2022-03-01 18:09 ` jaydeepjd.8914
2022-03-01 19:59 ` Johannes Sixt
2022-03-01 19:47 ` Johannes Sixt [this message]
2022-03-02 6:45 ` [GSoC][PATCHv2] userdiff: add builtin driver for kotlin language Jaydeep P Das
2022-03-02 6:45 ` [PATCH] " Jaydeep P Das
2022-03-02 8:00 ` Johannes Sixt
2022-03-02 9:09 ` jaydeepjd.8914
2022-03-02 9:28 ` jaydeepjd.8914
2022-03-02 14:26 ` [GSoC][PATCHv3] " Jaydeep P Das
2022-03-02 14:26 ` [PATCH] " Jaydeep P Das
2022-03-02 20:18 ` Johannes Sixt
2022-03-03 11:41 ` Jaydeep Das
2022-03-03 16:54 ` Ævar Arnfjörð Bjarmason
2022-03-03 19:47 ` Junio C Hamano
2022-03-03 20:04 ` Johannes Sixt
2022-03-04 12:28 ` Jaydeep Das
2022-03-04 13:59 ` Johannes Sixt
2022-03-03 18:15 ` [PATCH] userdiff: add builtin diff driver for Kotlin language Jaydeep P Das
2022-03-04 2:44 ` Junio C Hamano
2022-03-04 5:16 ` jaydeepjd.8914
2022-03-04 7:25 ` Johannes Sixt
2022-03-05 9:40 ` [PATCH v4] " Jaydeep P Das
2022-03-05 14:17 ` Johannes Sixt
2022-03-05 19:18 ` jaydeepjd.8914
2022-03-05 22:17 ` Johannes Sixt
2022-03-06 11:15 ` [PATCH v5] userdiff: add builtin diff driver for kotlin language Jaydeep P Das
2022-03-07 7:07 ` Johannes Sixt
2022-03-08 16:54 ` jaydeepjd.8914
2022-03-08 18:32 ` Johannes Sixt
2022-03-10 10:52 ` jaydeepjd.8914
2022-03-10 16:29 ` Jaydeep Das
2022-03-10 19:11 ` Johannes Sixt
2022-03-11 7:27 ` [PATCH v6] " Jaydeep P Das
2022-03-11 20:07 ` Johannes Sixt
2022-03-12 4:36 ` jaydeepjd.8914
2022-03-12 8:36 ` Johannes Sixt
2022-03-12 4:48 ` [PATCH v7] " Jaydeep P Das
2022-03-12 8:59 ` Johannes Sixt
2022-03-13 17:02 ` jaydeepjd.8914
2022-03-13 17:09 ` jaydeepjd.8914
2022-03-13 21:36 ` Johannes Sixt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5df2c9ce-b243-0173-befb-e13a6a74e387@kdbg.org \
--to=j6t@kdbg.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jaydeepjd.8914@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).