From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Rodrigo Carvalho <rodrigorsdc@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] userdiff: add support for R programming language
Date: Tue, 27 May 2025 08:04:14 -0700 [thread overview]
Message-ID: <xmqq4ix6rt75.fsf@gitster.g> (raw)
In-Reply-To: <385d7189-98fa-4863-8bcf-905c76eccbc8@kdbg.org> (Johannes Sixt's message of "Mon, 26 May 2025 07:57:33 +0200")
Johannes Sixt <j6t@kdbg.org> writes:
>> + "^[ \t]*([a-zA-z][a-zA-Z0-9_.]*[ \t]*<-[ \t]*function.*)$",
>
> I wonder how useful this is in practice. Unlike C or Java for example,
> code can live outside of functions in R scripts. If you have a script
> without any functions, there would not be any hunk headers. If you have
> a script with a mix of functions and code outside of functions, the code
> after a function would be attributed to the function. I'm not saying
> that this is bad, but just asking if this is part of the plan.
Isn't it the same as shell, perl, python, e-lisp and perhaps others?
If we can reliably detect that we are outside of any function and
set it to an empty string that would be great ;-).
>> + /* -- */
>> + "[a-zA-Z_][a-zA-Z0-9_.]*"),
>
> This singles out identifiers. Every single other characters would be its
> own word. I'd consider this a disimprovement. If you are not prepared to
> provide worddiff patterns, I recommend to use "[^ \t]+", which roughly
> amounts to the default behavior. It can be improved incrementally in
> later patches.
Good point.
> Please squash the test cases into this patch. Don't forget to test an
> indented function, and while at it, test a function definition *nested*
> in a function definition: that documents what the expected outcome is.
Again, good point.
Thanks.
next prev parent reply other threads:[~2025-05-27 15:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-25 21:02 [PATCH 1/2] userdiff: add support for R programming language Rodrigo Carvalho
2025-05-25 21:02 ` [PATCH 2/2] t4018: add two userdiff tests for R language Rodrigo Carvalho
2025-05-25 22:42 ` Eric Sunshine
2025-05-26 5:57 ` [PATCH 1/2] userdiff: add support for R programming language Johannes Sixt
2025-05-27 15:04 ` Junio C Hamano [this message]
2025-05-29 21:36 ` Rodrigo Carvalho
2025-05-29 22:16 ` [PATCH v2] " Rodrigo Carvalho
2025-06-03 18:20 ` Junio C Hamano
2025-06-03 21:15 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4ix6rt75.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=j6t@kdbg.org \
--cc=rodrigorsdc@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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.