From db08cb2888e469c8ce7bc289469c5004fe2d0eb3 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 17 Sep 2024 18:04:56 +0200 Subject: [PATCH 1/5] Improve formatting of annotated bindings in Nickel This commit fixes some indentation issue with annotated let-bindings (https://github.com/tweag/topiary/issues/743). All the proposals of said issue couldn't be implemented in current Topiary, so this only takes care of the `in` placement. This commit refactors a bit the associated queries and tries to improve documentation. --- topiary-queries/queries/nickel.scm | 77 +++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/topiary-queries/queries/nickel.scm b/topiary-queries/queries/nickel.scm index a9f6a4ec..b97b1075 100644 --- a/topiary-queries/queries/nickel.scm +++ b/topiary-queries/queries/nickel.scm @@ -318,19 +318,15 @@ ;; Annotations -; Start an indentation block from the start of the annotations to the -; end of the enclosing node -(_ - (annot) @prepend_indent_start -) @append_indent_end - -; Start a scope from the node previous to the annotations. -; This properly checks if the annotations were intended to be -; on newlines in such cases as: +; Start a scope from the node previous to the annotations. This properly checks +; if the annotations were intended to be on newlines in such cases as: +; ; id ; | a -> a +; ; which, without the annotations scope, would consider the annotations to be a ; single line node and format it as such: +; ; id | a -> a ( (#scope_id! "annotations") @@ -339,17 +335,74 @@ (annot) @append_end_scope ) -; Put each annotation -- and the equals sign, if it follows annotations -; -- on a new line, in a multi-line context. +; Put each annotation on a new line, in a multi-line context. (annot (#scope_id! "annotations") (annot_atom) @prepend_spaced_scoped_softline ) +; Add a new line before the last annotation and the following equal sign. +; +; [^annotations-followed-by-eq]: Ideally, we would like to only add this new +; line for multi-line annotations only. That is, we would like to have the +; following formatting: +; +; let foo +; | Array Number +; | doc "hello" +; = [ +; 1, +; 2, +; ] +; in ... +; +; But +; +; let foo | Array Number = [ +; 1, +; 2, +; ] +; in ... +; +; While adding a scoped line isn't an issue, note that in the examples above, +; the indentation of what comes after the `=` sign depends on the multi-liness +; of the annotations (and thus of the multiliness of the "annotations" scope). +; However, the RHS isn't part of this scope (and we don't want it to be). +; Unfortunately, this can't be achieved in current Topiary. +; +; In the meantime, we always put the `=` sign a new line, whether in single-line +; or multi-line mode, and always indent the RHS further in presence of +; annotations. This give the following formatting for the second example: +; +; let foo | Array Number +; = [ +; 1, +; 2, +; ] +; in ... +; +; which isn't optimal but still acceptable. +( + (annot) @append_spaced_softline + . + "=" +) + +; Indent the annotations with respect to the identifier they annotate. ( + (annot) @prepend_indent_start @append_indent_end +) + +; Indent the RHS of the let-binding in presence of annotations. +; +; Ideally, we would like to indent only when annotations are multi-line, but +; this isn't current possible; see [^annotations-followed-by-eq]. +(_ (annot) . - "=" @prepend_spaced_softline + "=" @prepend_indent_start + . + (_) @append_indent_end ) ; Break a multi-line polymorphic type annotation after the type From b06bc33fb616f4702c6b869ea3007bf8b11fe9e4 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Thu, 19 Sep 2024 14:43:17 +0200 Subject: [PATCH 2/5] Include comments in the post-annot indent block --- topiary-queries/queries/nickel.scm | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/topiary-queries/queries/nickel.scm b/topiary-queries/queries/nickel.scm index b97b1075..f956f8ea 100644 --- a/topiary-queries/queries/nickel.scm +++ b/topiary-queries/queries/nickel.scm @@ -398,11 +398,9 @@ ; Ideally, we would like to indent only when annotations are multi-line, but ; this isn't current possible; see [^annotations-followed-by-eq]. (_ - (annot) - . - "=" @prepend_indent_start - . - (_) @append_indent_end + (annot) @append_indent_start + "=" + (term) @append_indent_end ) ; Break a multi-line polymorphic type annotation after the type From 76f314ebe87d682c9d095e602cd6095571d41f60 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Thu, 19 Sep 2024 14:46:59 +0200 Subject: [PATCH 3/5] Add regression test --- topiary-cli/tests/samples/expected/nickel.ncl | 15 ++++++++++++++- topiary-cli/tests/samples/input/nickel.ncl | 16 +++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/topiary-cli/tests/samples/expected/nickel.ncl b/topiary-cli/tests/samples/expected/nickel.ncl index 892c1a78..5407fb33 100644 --- a/topiary-cli/tests/samples/expected/nickel.ncl +++ b/topiary-cli/tests/samples/expected/nickel.ncl @@ -139,7 +139,20 @@ 1 else 3 - ) + ), + + # regression test for https://github.com/tweag/topiary/issues/743 + # (partially fixed) + let foo + | Number + = [ + 1, + 2, + 3 + ] + in + foo + @ [], ], # Nickel standard library as of 44aef1672a09a76a71946fbf822713747ab7b9df diff --git a/topiary-cli/tests/samples/input/nickel.ncl b/topiary-cli/tests/samples/input/nickel.ncl index 14acfe0f..05a86a50 100644 --- a/topiary-cli/tests/samples/input/nickel.ncl +++ b/topiary-cli/tests/samples/input/nickel.ncl @@ -127,7 +127,21 @@ bar == 'Goodbye if x == 1 then 1 else - 3) + 3), + + # regression test for https://github.com/tweag/topiary/issues/743 + # (partially fixed) + let foo + | Number + = +[ + 1, + 2, + 3 +] +in +foo +@ [], ], # Nickel standard library as of 44aef1672a09a76a71946fbf822713747ab7b9df From bdfb6836c1b734054618211c36b1a4c06549cebc Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Thu, 19 Sep 2024 14:48:48 +0200 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 673d6a84..794443ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ This name should be decided amongst the team before the release. ### Fixed - [#720](https://github.com/tweag/topiary/pull/720) [#722](https://github.com/tweag/topiary/pull/722) [#723](https://github.com/tweag/topiary/pull/723) [#724](https://github.com/tweag/topiary/pull/724) [#735](https://github.com/tweag/topiary/pull/735) [#738](https://github.com/tweag/topiary/pull/738) [#739](https://github.com/tweag/topiary/pull/739) [#745](https://github.com/tweag/topiary/pull/745) Various OCaml improvements +- [#744](https://github.com/tweag/topiary/pull/744) Nickel: fix the indentation of `in` for annotated multiline let-bindings ### Changed - [#704](https://github.com/tweag/topiary/pull/704) Refactors our postprocessing code to be more versatile. From 112350b0f10454aecc30ac3975043b9f1673f7a6 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Thu, 19 Sep 2024 15:44:25 +0200 Subject: [PATCH 5/5] Update topiary-queries/queries/nickel.scm --- topiary-queries/queries/nickel.scm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/topiary-queries/queries/nickel.scm b/topiary-queries/queries/nickel.scm index f956f8ea..012a28b7 100644 --- a/topiary-queries/queries/nickel.scm +++ b/topiary-queries/queries/nickel.scm @@ -343,7 +343,7 @@ ; Add a new line before the last annotation and the following equal sign. ; -; [^annotations-followed-by-eq]: Ideally, we would like to only add this new +; [^annotations-followed-by-eq]: Ideally, we would like to add this new ; line for multi-line annotations only. That is, we would like to have the ; following formatting: ;