Support for OTP 27 Sigils & Triple Quoted Strings #358
Conversation
facebook-github-bot
added
the
CLA Signed
Sigils: https://www.erlang.org/blog/highlights-otp-27/#sigils Triple Quoted Strings: https://www.erlang.org/blog/highlights-otp-27/#triple-quoted-strings Also adds tests for documentation attributes. See: https://www.erlang.org/blog/highlights-otp-27/#overhauled-documentation-system Resolves #355
|
Lovely! I had just started on a similar PR myself 😄 Does it support triple quoted strings in documentation attributes? I didn't see a specific test case about that: -module(foo).
-moduledoc """
The module.
""".
-doc """
The function.
Long description.
""".
f() -> ok.Also, does it re-indent triple quoted strings. E.g. from: f() ->
"""
the
long
string
""".to f() ->
"""
the
long
string
""".I used the following test module -module(triple_quoted_strings).
-moduledoc """
abc
""".
-doc "foobar".
-type my_type :: integer().
-doc """
baz
extra
""".
-opaque otype :: integer().
-doc "qux"
foo() ->
"""
foo
""".
bar() ->
"""
the
long
string
""".Together with the desired result -module(triple_quoted_strings).
-moduledoc """
abc
""".
-doc "foobar".
-type my_type :: integer().
-doc """
baz
extra
""".
-opaque otype :: integer().
-doc "qux"
foo() ->
"""
foo
""".
bar() ->
"""
the
long
string
""".as a test driver, though I'm not 100% sure about if the result will be correct since I never finished implementing the indentation logic. |
Yes.
No, I haven't looked at indentation. But that would be a good feature to have. I don't have the time right now to also get into this myself, but I would be very happy to collaborate to create a shared PR combined with your indentation logic. Do you want to send a PR to my PR branch? :D |
Ah, missed that. My bad!
Yeah, I can do that. I have the test and some "attempt" at getting indentation working. So far I added this hack but I have no clue if it is the right way to proceed or not: diff --git a/src/erlfmt_algebra.erl b/src/erlfmt_algebra.erl
index 0f0994c..29ea433 100644
--- a/src/erlfmt_algebra.erl
+++ b/src/erlfmt_algebra.erl
@@ -522,6 +522,9 @@ format(Width, _, [{Indent, _, #doc_line{count = Count}} | T]) ->
[NewLines, indent(Indent) | format(Width, Indent, T)];
format(Width, Column, [{Indent, M, #doc_cons{left = X, right = Y}} | T]) ->
format(Width, Column, [{Indent, M, X}, {Indent, M, Y} | T]);
+format(Width, Column, [{Indent, _, #doc_string{string = [$",$",$"|_] = S, length = L}} | T]) ->
+ [First|Rest] = string:split(S, <<"\n">>, all),
+ [First, [[indent(Column - string:length(First) - 1), R] || R <- Rest] | format(Width, Column, T)];
format(Width, Column, [{_, _, #doc_string{string = S, length = L}} | T]) ->
[S | format(Width, Column + L, T)];
format(Width, Column, [{_, _, S} | T]) when is_binary(S) ->Some feedback on that direction from you or the maintainers would be nice 😅 |
|
Thank you for the PR, this is great! I agree we do need to handle indentation of the tripple-quote strings before merging. The test example that @eproxus gave is a good one to include, however I see an inconsistency with: -moduledoc """
abc
""".
-doc """
baz
extra
""".I believe both should end up indented like the -moduledoc """
abc
""".
-doc """
baz
extra
""".As to the particular approach, I'm generally fine with a hacky solution - the formatter has many of those already 😅 |
|
@michalmuskala I added the second in the sense of that the tool should try to keep the existing desire of the user if possible (as is done with line breaks somewhat). But perhaps for indentation that rule is not really applied now that I think about it... |
|
@eproxus in general we retain line breaks introduced by the user, but we do re-indent everything according to the rules |
|
@michalmuskala Makes sense! @maennchen Do you want a PR with my stuff right now to your branch, or would you rather fix the comments first? |
|
@eproxus Fixes are done. Go ahead :) |
|
Is this ready to merge? Or should I wait for the work on indentation? |
|
I think the PR is good enough to add compatibility for OTP 27. I got stuck in the rabbit whole of actually implementing indentation and didn't succeed yet. @maennchen Do you want my half-working code there? |
|
@michalmuskala We can merge this without indentation support. 👍 @eproxus I don't plan on working on the indentation. But if anyone else wants to, it might be good to push the code somewhere so that they can look at the work you already did. |
|
I'll look into properly handling indentation myself Thank you both for working on this! ❤️ |
|
@maennchen I noticed you implemented conversion of single-line tripple-quoted strings into a regular string. I'm not sure we should actually do that - in general we try to preserve the "style" of an expression the user wrote. I'm leaning towards removing this conversion |
|
Support for indentation was implemented in #362 and I cut a 1.5.0 release with support for the new OTP 27 features |
Sigils: https://www.erlang.org/blog/highlights-otp-27/#sigils
Triple Quoted Strings: https://www.erlang.org/blog/highlights-otp-27/#triple-quoted-strings
Also adds tests for documentation attributes. See: https://www.erlang.org/blog/highlights-otp-27/#overhauled-documentation-system
Resolves #355