Skip to content

Commit 7b1c316

Browse files
authored
Merge pull request #768 from tweag/fix/nickel-let-annotated-2
Nickel: formatting of single-line annotated let
2 parents aef207e + ffb44c9 commit 7b1c316

File tree

4 files changed

+100
-36
lines changed

4 files changed

+100
-36
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ This name should be decided amongst the team before the release.
5353
### Fixed
5454
- [#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) [#755](https://github.com/tweag/topiary/pull/755) [#759](https://github.com/tweag/topiary/pull/759) [#764](https://github.com/tweag/topiary/pull/764 Various OCaml improvements
5555
- [#762](https://github.com/tweag/topiary/pull/762) Various Rust improvements
56-
- [#744](https://github.com/tweag/topiary/pull/744) Nickel: fix the indentation of `in` for annotated multiline let-bindings
56+
- [#744](https://github.com/tweag/topiary/pull/744) [#768](https://github.com/tweag/topiary/pull/768) Nickel: fix the formatting of annotated multiline let-bindings
5757
- [#761](https://github.com/tweag/topiary/pull/761) No longer use error code 1 for unspecified errors
5858

5959
### Changed

topiary-cli/tests/samples/expected/nickel.ncl

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@
141141
3
142142
),
143143

144-
# regression test for https://github.com/tweag/topiary/issues/743
145-
# (partially fixed)
144+
# regression tests for https://github.com/tweag/topiary/issues/743
146145
let foo
147146
| Number
148147
= [
@@ -153,6 +152,36 @@
153152
in
154153
foo
155154
@ [],
155+
let x | Number = 1 + 1 in x,
156+
let x | Number = 1 + 1
157+
in x,
158+
let x | Number | std.number.PosNat =
159+
1
160+
+ 1
161+
in x,
162+
let x | Number = 1 + 1
163+
in
164+
x,
165+
let x | Array String = ["a", "b"]
166+
in x,
167+
let x | Array Number = [
168+
1,
169+
2,
170+
3
171+
]
172+
in
173+
x,
174+
let x
175+
| Array Number
176+
= [
177+
1 + 1,
178+
2 + 2,
179+
3 + 3,
180+
]
181+
in
182+
x,
183+
184+
# let blocks
156185
let x = 1, y = 2 in x + y,
157186
let
158187
x = 1,

topiary-cli/tests/samples/input/nickel.ncl

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,7 @@ bar == 'Goodbye
129129
else
130130
3),
131131

132-
# regression test for https://github.com/tweag/topiary/issues/743
133-
# (partially fixed)
132+
# regression tests for https://github.com/tweag/topiary/issues/743
134133
let foo
135134
| Number
136135
=
@@ -143,6 +142,45 @@ in
143142
foo
144143
@ [],
145144

145+
let x | Number = 1 + 1 in x,
146+
let x | Number = 1 + 1
147+
in x,
148+
149+
let x | Number | std.number.PosNat = 1
150+
+ 1
151+
in x,
152+
153+
154+
let x | Number
155+
= 1 + 1
156+
in
157+
x,
158+
159+
let x | Array String
160+
=
161+
["a","b"]
162+
in x,
163+
164+
let x | Array Number
165+
= [
166+
1,
167+
2,
168+
3
169+
]
170+
in
171+
x,
172+
173+
let x
174+
| Array Number
175+
=
176+
[1 + 1,
177+
2+2,
178+
3+3,]
179+
in
180+
x,
181+
182+
# let blocks
183+
146184
let x = 1, y = 2 in x + y,
147185

148186
let x = 1,

topiary-queries/queries/nickel.scm

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,9 @@
355355

356356
;; Annotations
357357

358-
; Start a scope from the node previous to the annotations. This properly checks
359-
; if the annotations were intended to be on newlines in such cases as:
358+
; Start a scope from the node preceding the annotations. This scope is used
359+
; exclusively to lay out annotations on multiple lines. The rule checks if the
360+
; annotations were intended to be on newlines as in:
360361
;
361362
; id
362363
; | a -> a
@@ -378,11 +379,24 @@
378379
(annot_atom) @prepend_spaced_scoped_softline
379380
)
380381

381-
; Add a new line before the last annotation and the following equal sign.
382+
; Start a new scope for annotations, used to properly indent any content coming
383+
; after the annotations. We use Topiary's measuring scope feature: the
384+
; multi-liness of this scope is entirely decided by the annotations only, but
385+
; what we want to affect is larger, namely the content of the binding coming
386+
; after the `=`.
387+
(
388+
(#scope_id! "annotations_with_content")
389+
(_) @append_begin_scope @append_begin_measuring_scope
390+
.
391+
(annot) @append_end_measuring_scope
392+
"="
393+
(term) @append_end_scope
394+
)
395+
396+
; Add a new line before the last annotation and the following equal sign, when
397+
; the annotations are multi-line.
382398
;
383-
; [^annotations-followed-by-eq]: Ideally, we would like to add this new
384-
; line for multi-line annotations only. That is, we would like to have the
385-
; following formatting:
399+
; Indeed, we want to have the following formatting (multi-line):
386400
;
387401
; let foo
388402
; | Array Number
@@ -393,34 +407,16 @@
393407
; ]
394408
; in ...
395409
;
396-
; But
410+
; But (single-line):
397411
;
398412
; let foo | Array Number = [
399413
; 1,
400414
; 2,
401415
; ]
402416
; in ...
403-
;
404-
; While adding a scoped line isn't an issue, note that in the examples above,
405-
; the indentation of what comes after the `=` sign depends on the multi-liness
406-
; of the annotations (and thus of the multiliness of the "annotations" scope).
407-
; However, the RHS isn't part of this scope (and we don't want it to be).
408-
; Unfortunately, this can't be achieved in current Topiary.
409-
;
410-
; In the meantime, we always put the `=` sign a new line, whether in single-line
411-
; or multi-line mode, and always indent the RHS further in presence of
412-
; annotations. This give the following formatting for the second example:
413-
;
414-
; let foo | Array Number
415-
; = [
416-
; 1,
417-
; 2,
418-
; ]
419-
; in ...
420-
;
421-
; which isn't optimal but still acceptable.
422417
(
423-
(annot) @append_spaced_softline
418+
(#scope_id! "annotations_with_content")
419+
(annot) @append_spaced_scoped_softline
424420
.
425421
"="
426422
)
@@ -430,11 +426,12 @@
430426
(annot) @prepend_indent_start @append_indent_end
431427
)
432428

433-
; Indent the RHS of the let-binding in presence of annotations.
434-
;
435-
; Ideally, we would like to indent only when annotations are multi-line, but
436-
; this isn't current possible; see [^annotations-followed-by-eq].
429+
; Indent the bound expression of a let-binding (or a field definition) in
430+
; presence of multi-line annotations. That's where we use the measuring scope of
431+
; "annotations_with_content": we want this rule to fire only when (annot) is
432+
; multi-line, regardless of the multi-liness of the ("=" (term)) part.
437433
(_
434+
(#multi_line_scope_only! "annotations_with_content")
438435
(annot) @append_indent_start
439436
"="
440437
(term) @append_indent_end

0 commit comments

Comments
 (0)