From: Johannes Sixt <j6t@kdbg.org>
To: "Łukasz Niemier" <lukasz@niemier.pl>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3] userdiff: add Elixir to supported userdiff languages
Date: Thu, 7 Nov 2019 21:35:01 +0100 [thread overview]
Message-ID: <f85a5c31-52ff-85be-b1e0-f3982fd2a7f2@kdbg.org> (raw)
In-Reply-To: <20191106234941.48926-1-lukasz@niemier.pl>
Please keep the Cc list when you send new patch versions. Also, it is
customary to send them as replies to the earlier iterations, so that
they all end up in the same thread.
Am 07.11.19 um 00:49 schrieb Łukasz Niemier:
> Adds support for xfuncref in Elixir[1] language which is Ruby-like
> language that runs on Erlang[3] Virtual Machine (BEAM).
>
> [1]: https://elixir-lang.org
> [2]: https://www.erlang.org
Thanks! Much appreciated.
>
> Signed-off-by: Łukasz Niemier <lukasz@niemier.pl>
> ---
> diff --git a/t/t4018/elixir-do-not-pick-end b/t/t4018/elixir-do-not-pick-end
> new file mode 100644
> index 0000000000..fae08ba7e8
> --- /dev/null
> +++ b/t/t4018/elixir-do-not-pick-end
> @@ -0,0 +1,5 @@
> +defmodule RIGHT do
> +end
> +#
> +#
> +# ChangeMe; do not pick up 'end' line
OK.
> diff --git a/t/t4018/elixir-ex-unit-test b/t/t4018/elixir-ex-unit-test
> new file mode 100644
> index 0000000000..0560a2b697
> --- /dev/null
> +++ b/t/t4018/elixir-ex-unit-test
> @@ -0,0 +1,6 @@
> +defmodule Test do
> + test "RIGHT" do
> + assert true == true
> + assert ChangeMe
> + end
> +end
A test, and also indented. Good.
> diff --git a/t/t4018/elixir-module-func b/t/t4018/elixir-module-func
> new file mode 100644
> index 0000000000..c9910d0675
> --- /dev/null
> +++ b/t/t4018/elixir-module-func
> @@ -0,0 +1,8 @@
> +defmodule Foo do
> + def fun(RIGHT) do
> + # Code
> + # Code
> + # Code
> + ChangeMe
> + end
> +end
An indented function. Good.
These other tests (which I stripped away) ensure that the hunk header
pattern does not become too restrictive. They all look good.
> diff --git a/userdiff.c b/userdiff.c
> index e187d356f6..577053c10a 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -32,6 +32,18 @@ PATTERNS("dts",
> /* Property names and math operators */
> "[a-zA-Z0-9,._+?#-]+"
> "|[-+*/%&^|!~]|>>|<<|&&|\\|\\|"),
> +PATTERNS("elixir",
> + "^[ \t]*((def(macro|module|impl|protocol|p)?|test)[ \t].*)$",
> + /* Atoms, names, and module attributes */
> + "|[@:]?[a-zA-Z0-9@_?!]+"
There are no single- and double-quote anymore. An oversight? Or an error
in the earlier iteration?
> + /* Numbers with specific base */
> + "|[-+]?0[xob][0-9a-fA-F]+"
> + /* Numbers */
> + "|[-+]?[0-9][0-9_.]*([eE][-+]?[0-9_]+)?"
The leading optional sign may be problematic. When a patch changes "i+1"
to "i+2", it would be highlighted as "i{-+1-}{++2+}" instead of as
"i+{-1-}{+2+}". You could remove the leading optional sign and let it be
processed as an operator. But we also have an optional sign in the cpp
pattern as well, and haven't noticed it until now, so...
> + /* Operators and atoms that represent them */
> + "|:?(\\+\\+|--|\\.\\.|~~~|<>|\\^\\^\\^|<?\\|>|<<<?|>?>>|<<?~|~>?>|<~>|<=|>=|===?|!==?|=~|&&&?|\\|\\|\\|?|=>|<-|\\\\\\\\|->)"
> + /* Not real operators, but should be grouped */
> + "|:?%[A-Za-z0-9_.]\\{\\}?"),
> IPATTERN("fortran",
> "!^([C*]|[ \t]*!)\n"
> "!^[ \t]*MODULE[ \t]+PROCEDURE[ \t]\n"
>
In summary, this version looks good!
Acked-by: Johannes Sixt <j6t@kdbg.org>
-- Hannes
next prev parent reply other threads:[~2019-11-07 20:35 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-06 23:49 [PATCH v3] userdiff: add Elixir to supported userdiff languages Łukasz Niemier
2019-11-07 20:35 ` Johannes Sixt [this message]
2019-11-08 15:42 ` Łukasz Niemier
2019-11-08 21:38 ` [PATCH v4] " Łukasz Niemier
2019-11-09 19:31 ` 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=f85a5c31-52ff-85be-b1e0-f3982fd2a7f2@kdbg.org \
--to=j6t@kdbg.org \
--cc=git@vger.kernel.org \
--cc=lukasz@niemier.pl \
/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).