{"thread":{"id":"52204","subject":"[PATCH v2] feat: add Elixir to supported userdiff languages","startedAt":"2019-11-06T17:48:52Z","lastAt":"2019-11-06T22:07:57Z","messageCount":3,"participants":["Łukasz Niemier","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"385613","messageId":"20191106174844.23482-1-lukasz@niemier.pl","threadId":"52204","inReplyTo":null,"subject":"[PATCH v2] feat: add Elixir to supported userdiff languages","fromName":"Łukasz Niemier","fromEmail":"lukasz@niemier.pl","sentAt":"2019-11-06T17:48:44Z","receivedAt":"2019-11-06T17:48:52Z","isPatch":true,"sender":{"key":"lukasz@niemier.pl","avatar":"https://avatars.githubusercontent.com/u/291639?v=4"},"body":"---\nRemove file that landed accidentally in the commit\n t/t4018-diff-funcname.sh | 1 +\n t/t4018/elixir-function  | 5 +++++\n t/t4018/elixir-module    | 9 +++++++++\n userdiff.c               | 7 +++++++\n 4 files changed, 22 insertions(+)\n create mode 100644 t/t4018/elixir-function\n create mode 100644 t/t4018/elixir-module\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 6f5ef0035e..194310377e 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -31,6 +31,7 @@ diffpatterns=\"\n \tcpp\n \tcsharp\n \tcss\n+\telixir\n \tdts\n \tfortran\n \tfountain\ndiff --git a/t/t4018/elixir-function b/t/t4018/elixir-function\nnew file mode 100644\nindex 0000000000..d452f495a7\n--- /dev/null\n+++ b/t/t4018/elixir-function\n@@ -0,0 +1,5 @@\n+def function(RIGHT, arg) do\n+  # comment\n+  # comment\n+  ChangeMe\n+end\ndiff --git a/t/t4018/elixir-module b/t/t4018/elixir-module\nnew file mode 100644\nindex 0000000000..91a4e7aa20\n--- /dev/null\n+++ b/t/t4018/elixir-module\n@@ -0,0 +1,9 @@\n+defmodule RIGHT do\n+  @moduledoc \"\"\"\n+  Foo bar\n+  \"\"\"\n+\n+  def ChangeMe(a) where is_map(a) do\n+    a\n+  end\n+end\ndiff --git a/userdiff.c b/userdiff.c\nindex e187d356f6..31fff34e1e 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -32,6 +32,13 @@ PATTERNS(\"dts\",\n \t /* Property names and math operators */\n \t \"[a-zA-Z0-9,._+?#-]+\"\n \t \"|[-+*/%&^|!~]|>>|<<|&&|\\\\|\\\\|\"),\n+PATTERNS(\"elixir\",\n+\t \"^[ \\t]*((def(macro|module|impl|guard|protocol)?p?|test)[ \\t].*)$\",\n+\t \"[a-zA-Z0-9_.]+\"\n+\t \"|:[a-zA-Z0-9@_]+\"\n+\t \"|:'a-zA-Z0-9@_]+'\"\n+\t \"|:\\\"[a-zA-Z0-9@_]+\\\"\"\n+\t \"|@[a-zA-Z0-9_]+\"),\n IPATTERN(\"fortran\",\n \t \"!^([C*]|[ \\t]*!)\\n\"\n \t \"!^[ \\t]*MODULE[ \\t]+PROCEDURE[ \\t]\\n\"\n-- \n2.23.0\n\n"},{"id":"385622","messageId":"672bdea4-180a-89e0-c420-df2704ed887e@kdbg.org","threadId":"52204","inReplyTo":"20191106174844.23482-1-lukasz@niemier.pl","subject":"Re: [PATCH v2] feat: add Elixir to supported userdiff languages","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-11-06T20:39:16Z","receivedAt":"2019-11-06T20:39:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Thank you for your contribution!\n\nAm 06.11.19 um 18:48 schrieb Łukasz Niemier:\n> ---\n\nPlease use \"subsystem: short description\" in the subject. For example:\n\n   userdiff: support Elixir\n\nwould be sufficient in this case.\n\nPlease add your sign-off before the three-dash line so that we know that\nyou are entitled to publish this patch. See Documentation/SubmittingPatches.\n\nIt would be enlightening to know what Elixir is. (I haven't googled it,\nyet.) If it were a popular language, I think I would have heard about\nit. But it may well be possible that I have lived under a rock for too\nlong... ;)\n\n> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\n> index 6f5ef0035e..194310377e 100755\n> --- a/t/t4018-diff-funcname.sh\n> +++ b/t/t4018-diff-funcname.sh\n> @@ -31,6 +31,7 @@ diffpatterns=\"\n>  \tcpp\n>  \tcsharp\n>  \tcss\n> +\telixir\n>  \tdts\n>  \tfortran\n>  \tfountain\n\nThis list is sorted, basically, but your addition perturbates the order.\n\n> diff --git a/t/t4018/elixir-function b/t/t4018/elixir-function\n> new file mode 100644\n> index 0000000000..d452f495a7\n> --- /dev/null\n> +++ b/t/t4018/elixir-function\n> @@ -0,0 +1,5 @@\n> +def function(RIGHT, arg) do\n> +  # comment\n> +  # comment\n> +  ChangeMe\n> +end\n> diff --git a/t/t4018/elixir-module b/t/t4018/elixir-module\n> new file mode 100644\n> index 0000000000..91a4e7aa20\n> --- /dev/null\n> +++ b/t/t4018/elixir-module\n> @@ -0,0 +1,9 @@\n> +defmodule RIGHT do\n> +  @moduledoc \"\"\"\n> +  Foo bar\n> +  \"\"\"\n> +\n> +  def ChangeMe(a) where is_map(a) do\n> +    a\n> +  end\n> +end\n\nThe default hunk header pattern picks up lines that begin with a letter\nwithout leading whitespace. The tests that you present here do not show\nthat the language specific hunk header pattern is better. The default\nwould have picked up the correct lines. And, in fact, when I remove the\npattern from the code, these tests still pass!\n\nI'm not saying that the pattern is bad; I say that the tests do not show\nits worthiness. More tests are needed. For example:\n\n--- 8< ---\ndefmodule RIGHT do\nend\n#\n#\n# ChangeMe; do not pick up 'end' line\n--- 8< ---\n\nBTW, I guess that any def, defmodule, etc. as the first word on a line\nin the docstring would be picked up incorrectly. Is that a problem?\n\n> diff --git a/userdiff.c b/userdiff.c\n> index e187d356f6..31fff34e1e 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -32,6 +32,13 @@ PATTERNS(\"dts\",\n>  \t /* Property names and math operators */\n>  \t \"[a-zA-Z0-9,._+?#-]+\"\n>  \t \"|[-+*/%&^|!~]|>>|<<|&&|\\\\|\\\\|\"),\n> +PATTERNS(\"elixir\",\n> +\t \"^[ \\t]*((def(macro|module|impl|guard|protocol)?p?|test)[ \\t].*)$\",\n\nGood. A test case that shows that indented def lines are picked up, if\nthat is the intent, would be appreciated.\n\n> +\t \"[a-zA-Z0-9_.]+\"\n> +\t \"|:[a-zA-Z0-9@_]+\"\n> +\t \"|:'a-zA-Z0-9@_]+'\"\n\nThe opening bracket is missing here.\n\n> +\t \"|:\\\"[a-zA-Z0-9@_]+\\\"\"\n> +\t \"|@[a-zA-Z0-9_]+\"),\nWould it be an option to collapse all but the first pattern (because I\ndo not want to start the pattern with an optional part) to\n\n\t\"[:@]['\\\"]?[a-zA-Z0-9@_]\"\n\nThis assumes that @\"x1 and @'y2 cannot occur in a syntactically valid\nprogram. Remember: the patterns can be loose; they do not have to\nvalidate the input, but can assume that it is syntactically valid.\n\nDoes the language not have any two-character operators, such as '<='?\n\n>  IPATTERN(\"fortran\",\n>  \t \"!^([C*]|[ \\t]*!)\\n\"\n>  \t \"!^[ \\t]*MODULE[ \\t]+PROCEDURE[ \\t]\\n\"\n> \n\n-- Hannes\n"},{"id":"385629","messageId":"ED332708-76D5-447F-A2D7-6495CC6A76DC@niemier.pl","threadId":"52204","inReplyTo":"672bdea4-180a-89e0-c420-df2704ed887e@kdbg.org","subject":"Re: [PATCH v2] feat: add Elixir to supported userdiff languages","fromName":"Łukasz Niemier","fromEmail":"lukasz@niemier.pl","sentAt":"2019-11-06T22:07:50Z","receivedAt":"2019-11-06T22:07:57Z","isPatch":true,"sender":{"key":"lukasz@niemier.pl","avatar":"https://avatars.githubusercontent.com/u/291639?v=4"},"body":"> Please use \"subsystem: short description\" in the subject. For example:\n> \n>   userdiff: support Elixir\n> \n> would be sufficient in this case.\n\nOk, will fix.\n\n> Please add your sign-off before the three-dash line so that we know that\n> you are entitled to publish this patch. See Documentation/SubmittingPatches.\n\nYeah, I have seen it too late to fix. Will do.\n\n> It would be enlightening to know what Elixir is. (I haven't googled it,\n> yet.)\n\nIt is language with Ruby-like syntax for BEAM (Erlang virtual machine).\n\nhttps://elixir-lang.org\n\n> If it were a popular language, I think I would have heard about\n> it. But it may well be possible that I have lived under a rock for too\n> long... ;)\n\nIt is quite popular among few services (Pinterest, Discord, Bleacher Report),\nbut it is still pretty new, and still need to get a little more spotlight.\n\n> This list is sorted, basically, but your addition perturbates the order.\n\nOh, sorry, I missed dts.\n\n> The default hunk header pattern picks up lines that begin with a letter\n> without leading whitespace. The tests that you present here do not show\n> that the language specific hunk header pattern is better. The default\n> would have picked up the correct lines. And, in fact, when I remove the\n> pattern from the code, these tests still pass!\n> \n> I'm not saying that the pattern is bad; I say that the tests do not show\n> its worthiness. More tests are needed. For example:\n> \n> --- 8< ---\n> defmodule RIGHT do\n> end\n> #\n> #\n> # ChangeMe; do not pick up 'end' line\n> --- 8< —\n\nYeah, I will provide such\n\n> BTW, I guess that any def, defmodule, etc. as the first word on a line\n> in the docstring would be picked up incorrectly. Is that a problem?\n\nNo, as this would (almost?) always be the definition of the module/function,\nso it is not a problem at all.\n\n>> +\t \"|:'a-zA-Z0-9@_]+'\"\n> \n> The opening bracket is missing here.\n\nWhoops.\n\n> Would it be an option to collapse all but the first pattern (because I\n> do not want to start the pattern with an optional part) to\n> \n> \t\"[:@]['\\\"]?[a-zA-Z0-9@_]\"\n> \n> This assumes that @\"x1 and @'y2 cannot occur in a syntactically valid\n> program.\n\nNo, these aren’t valid.\n\n> Remember: the patterns can be loose; they do not have to\n> validate the input, but can assume that it is syntactically valid.\n\nOk\n\n> Does the language not have any two-character operators, such as '<=‚?\n\nIt has, I should add them as well (it even has 3 letter operators).\n\n--\n\nŁukasz Niemier\nlukasz@niemier.pl\n\n\n\n\n\n"}]}