clarinet format feedback

[tippenein]

The clarity formatter has been released in beta and we're looking for feedback. We're open to your ideas and bug reports. The LSP integration is coming next, but for now you can use the cli with clarinet fmt

[obycode]

This is great! I'm loving it, but just have a few issues I've noticed so far.

• The formatter drops the comment on this constant:

(define-constant WAITING_PERIOD u144) ;; 24 hours in blocks

• If I use the ;;; style for documentation comments, the formatter changes it to ;; ;

• I'm not sure I fully understand how it is deciding if two or more closing )s should be on the same line, or separate lines. This definitely seems like a personal preference thing, so I think I'd be okay with either option, but it seems inconsistent currently.

  • My personal preference would be that if they opened on the same line, they should close on the same line, otherwise, they should close on a separate line

[tippenein]

1 & 2. That's an oversight on my part. Should be an easy fix 👍
3. Yes, undoubtedly. The issue comes up when lines are width wrapped and we can't reliably tell where something started. We lose determinism of the output if we analyze the input start_line spans. We could do multiple passes (width wrap format, re-parse, back into format) but ideally we can come up with something more consistent which is still performant.

[hugocaillard]

I think multiple passes might be fine. You might even need "specialized" passes. Eventually with one dedicated to closing parenthesis that could be super performant

[tippenein]

The comment fixes are here: #1695

[cipherzzz]

I can confirm, this is fixed in latest release @tippenein

[pradel]

After running clarinet format on the Sigle contract, I have the following feedback. Note that you can see the diffs made by clarinet format on this pr (https://github.com/sigle/sigle/pull/1125/files)

• Bug: define-constant removes the ' in front of the contract principal
  

• Bug: define-data-var removes the value
  

• Bug: define-trait format not working Clarinet format with define-trait has unreadable output #1685

• Bug: seems like it kinda merged multiple functions on the same line
  

• Preference: after a define-map should have an empty line before the next define-public (line 29 that was removed)
  

• Preference: I added some line breaks to split the code and make it easier to understand but format is removing all of them

[tippenein]

Sounds reasonable to me! I'll add some tests for these. Will post the PR back here when it's ready. Thanks @pradel

[friedger]

' in front of the contract principal

This is also true for (contract-call?

[tippenein]

The example with print and unwrap-panic smashed together:

(begin
  (print { a: "publish-content", version: contract-version, minter: authorized-minter }))
  (unwrap-panic (as-contract (contract-call? .sigle-minter-fixed-price-v001 set-mint-details u0 u0 u1)))

I think this is a paren mistake? The formatter definitely didn't handle it well, but ideally it would looks like

(begin
  (print 
    { 
      a: "publish-content",
      version: contract-version, 
      minter: authorized-minter,
    })
)
(unwrap-panic (as-contract (contract-call? .sigle-minter-fixed-price-v001 set-mint-details u0 u0 u1)))  

Which isn't what you meant, I assume?

[pradel]

Yes, it seems to be a parenthesis mistake 👍

[tippenein]

I need to make it deal with those in a more graceful way 🫡

[friedger]

In general, I am for much less new lines.

The below means so much scrolling..

    ) 
  ) 
) 

And we save 1 char for each line.

[hugocaillard]

I think I agree on the "less new lines" parts. But could you provide more context to the snippet your shared @friedger?

This pattern you shared can be legit if the opening lines all require line breaks to not exceed max-line length (80 by default).
The closing parenthesis should just follow the opening one (i think we all agree on that - mostly). So it's more important to discuss line-break in the lines above the one you shared and see what are the line-break opportunities

[hugocaillard]

So to answer my own comment
(tl;dr is that: closing parentheses are just the consequences of opening ones so "less new lines" should be regarding opening lines)

The current formatter tends to maybe add to many new lines on nested fucntion calls, eg:

(ok
  (at-block
    (unwrap! (get-stacks-block-info? id-header-hash block) ERR_BLOCK_NOT_FOUND)
    (var-get count)
  )
)

The ok and at-block could be on the same line (less lines, less indentation)

(ok (at-block
  (unwrap! (get-stacks-block-info? id-header-hash block) ERR_BLOCK_NOT_FOUND)
  (var-get count)
))

[friedger]

I don't like values at the beginning of the line

[tippenein]

This looks like a candidate for ;; @format-ignore
If we indented those lines (assuming max line length is 80) then we'd wrap with 15 per line instead of 16
This is a bug in the wrapping logic fwiw, just thought I'd mention the formatting ignore special comment

[tippenein]

Opened an issue about the unintended behavior, although the solution is still likely @format-ignore

#1698

[hugocaillard]

I'd like to hear people opinion regarding one-lining or not function arguments and let bindings when there is only one argument or binding.
The current rule is to always put those on a new line, but I think it would make sense to have it on one line (if it fits within the max-line length).

---

Arguments

(define-public (add
    (n uint)
  )
  (ok (var-set count (+ (var-get count) n)))
)

vs

(define-public (add (n uint)) 
  (ok (var-set count (+ (var-get count) n)))
)

---

Let

(define-public (decrement)
  (let (
    (current-count (var-get count))
  )
    (asserts! (> current-count u0) ERR_COUNT_MUST_BE_POSITIVE)
    (ok (var-set count (- current-count u1)))
  )
)

vs

(define-public (decrement)
  (let ((current-count (var-get count)))
    (asserts! (> current-count u0) ERR_COUNT_MUST_BE_POSITIVE)
    (ok (var-set count (- current-count u1)))
  )
)

Note: If there are multiple arguments or let bindings, they should always be on new lines imo

[obycode]

Yes, I like the one-liner when there is one argument.

[cipherzzz]

I like the method signatures on one line and the let vars on one line. I think that the opening and closing brackets for logic in the body need to be on new lines and indented - otherwise clarity is unreadable

[tippenein]

@cipherzzz could you give an example of your ideal format? I think I understand, but want to make sure I'm getting it