Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent placement of in depending on the term being annotated #743

Closed
yannham opened this issue Sep 17, 2024 · 0 comments · Fixed by #744
Closed

Inconsistent placement of in depending on the term being annotated #743

yannham opened this issue Sep 17, 2024 · 0 comments · Fixed by #744

Comments

@yannham
Copy link
Member

yannham commented Sep 17, 2024

Describe the bug

In a multi-line let-binding, where the in keyword is put on a new line, the indentation of in depends on the presence of a type or a contract annotation on the bound variable, which looks a bit strange and inconsistent.

To Reproduce

Without annotation

let x = 1 + 1
in x

let x = 1 + 1
in
x

let x =
  1 + 1
in x

let x =
  1 + 1
in
x

With an annotation:

let x | Number
  = 1 + 1
  in x

let x | Number
  = 1 + 1
  in
x

let x | Number
  =
    1 + 1
  in x

let x | Number
  =
    1 + 1
  in
x

let x
  | Number
  =
    1 + 1
  in
x

Expected behavior

I see several issues with the current formatting of let-bindings with an annotation. I suppose the general layout idea is to mimic record fields in multiline mode, and to try to put the annotations and the = sign aligned as in the last example. However:

  1. Even if the = and the annotation are indented further to be aligned, we should not apply this additional indentation to the in keyword, which should always be indented at the same level as the original let:

    let x
      | Number
      =
        1 + 1
    in
    x
    
  2. We should either put both the = and the annotation on their own line, to align them, or let them be and not add any further indentation. The hybrid case, such as:

    let x | Number
      =
        1 + 1
      in
    x
    

    is strange because 1 + 1 seems unduly indented two times, as opposed to the case without an annotation. It should probably be just

    let x | Number =
      1 + 1
    in
    x
    

    as if the annotation was part of the identifier.

yannham added a commit that referenced this issue Sep 19, 2024
This commit fixes some indentation issue with annotated let-bindings
(#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.
yannham added a commit that referenced this issue Sep 19, 2024
This commit fixes some indentation issue with annotated let-bindings
(#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.
yannham added a commit that referenced this issue Oct 15, 2024
Previously, we were formatting a let-binding followed by a single-line
annotation and some content by always putting the `=` sign and the
following content on a new line, indenting the content one level to the
right.

This was mostly due to a limitation of Topiary around scopes. Today,
this limitation has been lifted thanks to the introduction of measuring
scopes. We make use of the latter to finally format such let-bindings
(and bindings in general) as we originally wanted (see
#743).
yannham added a commit that referenced this issue Oct 16, 2024
Previously, we were formatting a let-binding followed by a single-line
annotation and some content by always putting the `=` sign and the
following content on a new line, indenting the content one level to the
right.

This was mostly due to a limitation of Topiary around scopes. Today,
this limitation has been lifted thanks to the introduction of measuring
scopes. We make use of the latter to finally format such let-bindings
(and bindings in general) as we originally wanted (see
#743).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant