git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Atharva Raykar <raykar.ath@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [GSOC][PATCH] userdiff: add support for Scheme
Date: Sun, 28 Mar 2021 00:46:11 +0100	[thread overview]
Message-ID: <3def82fd-71a7-3ad9-0fa2-48598bfd3313@kdbg.org> (raw)
In-Reply-To: <20210327173938.59391-1-raykar.ath@gmail.com>

Am 27.03.21 um 18:39 schrieb Atharva Raykar:
>  - By best-effort attempt at the wordregex, I mean that it is a little
>    more permissive than it has to be, as it accepts a few words that are
>    technically invalid in Scheme.
>    Making it handle all cases like numbers and identifiers with separate
>    regexen would be greatly complicated (Eg: #x#e10.2f3 is a valid number
>    but #x#f10.2e3 is not; 10t1 is a valid identifier, but 10s1 is a number
>    -- my wordregex just clubs all of these into a generic 'word match' which
>    trades of granularity for simplicity, and it usually does the right thing).

It is ok to have regex that capture tokens that are not valid. A
userdiff driver can assume that it operates only text that is valid in
the language.

> diff --git a/t/t4018/scheme-define-syntax b/t/t4018/scheme-define-syntax
> new file mode 100644
> index 0000000000..603b99cea4
> --- /dev/null
> +++ b/t/t4018/scheme-define-syntax
> @@ -0,0 +1,8 @@
> +(define-syntax define-test-suite RIGHT
> +  (syntax-rules ()
> +    ((_ suite-name (name test) ChangeMe ...)
> +     (define suite-name

This test is suspicious. Notice the "ChangeMe" above? That is sufficient
to let the test case succeed. The "ChangeMe" in the last line below
should be the only one.

But then there is this indented '(define' that is not marked as RIGHT,
and I wonder how is it different from...

> +       (let ((tests
> +              `((name . ,test) ...)))
> +         (lambda ()
> +           (ChangeMe 'suite-name tests)))))))
> \ No newline at end of file
> diff --git a/t/t4018/scheme-local-define b/t/t4018/scheme-local-define
> new file mode 100644
> index 0000000000..90e75dcce8
> --- /dev/null
> +++ b/t/t4018/scheme-local-define
> @@ -0,0 +1,4 @@
> +(define (higher-order)
> +  (define local-function RIGHT

... this one, which is also indented and *is* marked as RIGHT.

BTW, it's good to see test cases for both indented and not-indented
trigger lines.

> +    (lambda (x)
> +     (car "this is" "ChangeMe"))))
> \ No newline at end of file

> diff --git a/userdiff.c b/userdiff.c
> index 3f81a2261c..c51a8c98ba 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -191,6 +191,14 @@ PATTERNS("rust",
>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>  	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"
>  	 "|[-+*\\/<>%&^|=!:]=|<<=?|>>=?|&&|\\|\\||->|=>|\\.{2}=|\\.{3}|::"),
> +PATTERNS("scheme",
> +         "^[\t ]*(\\(define-?.*)$",

This "optional hyphen followed by anything" in the regex is strange.
Wouldn't that also capture a line that looks like, e.g.,

    (defined-foo bar)

Perhaps we want "define[- \t].*" in the regex?

> +         /* 
> +          * Scheme allows symbol names to have any character,
> +          * as long as it is not a form of a parenthesis.
> +          * The spaces must be escaped.
> +          */
> +         "(\\.|[^][)(\\}\\{ ])+"),
>  PATTERNS("bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#}{~%]*).*$",
>  	 "[={}\"]|[^={}\" \t]+"),
>  PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
> 

-- Hannes

  parent reply	other threads:[~2021-03-27 23:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-27 17:39 [GSOC][PATCH] userdiff: add support for Scheme Atharva Raykar
2021-03-27 22:50 ` Junio C Hamano
2021-03-27 23:09   ` Junio C Hamano
2021-03-28  3:16     ` Ævar Arnfjörð Bjarmason
2021-03-28  5:37       ` Junio C Hamano
2021-03-28 12:40       ` Atharva Raykar
2021-03-29 10:08         ` Phillip Wood
2021-03-30  6:41           ` Atharva Raykar
2021-03-30 12:56             ` Ævar Arnfjörð Bjarmason
2021-03-30 13:48               ` Atharva Raykar
2021-03-28 12:45     ` Atharva Raykar
2021-03-28 11:51   ` Atharva Raykar
2021-03-28 18:06     ` Junio C Hamano
2021-03-29  8:12       ` Atharva Raykar
2021-03-29 20:47         ` Junio C Hamano
2021-03-29 10:12     ` Phillip Wood
2021-03-27 23:46 ` Johannes Sixt [this message]
2021-03-28 12:23   ` Atharva Raykar
2021-03-29 10:18     ` Phillip Wood
2021-03-29 10:48       ` Johannes Sixt
2021-03-29 13:12         ` Ævar Arnfjörð Bjarmason
2021-03-29 14:06           ` Phillip Wood
2021-03-30  7:04       ` Atharva Raykar
2021-03-30 10:22         ` Atharva Raykar
2021-04-05 10:04           ` Phillip Wood
2021-04-05 17:58             ` Johannes Sixt
2021-04-06 12:29             ` Atharva Raykar
2021-04-06 19:10               ` Phillip Wood
2021-04-03 13:16 ` [GSoC][PATCH v2 0/1] userdiff: add support for scheme Atharva Raykar
2021-04-03 13:16   ` [GSoC][PATCH v2 1/1] " Atharva Raykar
2021-04-05 10:21     ` Phillip Wood
2021-04-06 10:32       ` Atharva Raykar
2021-04-08  9:14   ` [GSoC][PATCH v3 0/1] " Atharva Raykar
2021-04-08  9:14   ` [GSoC][PATCH v3 1/1] userdiff: add support for Scheme Atharva Raykar
2021-04-12 23:04     ` Junio C Hamano

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=3def82fd-71a7-3ad9-0fa2-48598bfd3313@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=raykar.ath@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).