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

Adding Better Clojure Formatting from Tonsky #301

Open
ned-at-obrizum opened this issue Jun 14, 2023 · 8 comments
Open

Adding Better Clojure Formatting from Tonsky #301

ned-at-obrizum opened this issue Jun 14, 2023 · 8 comments

Comments

@ned-at-obrizum
Copy link

ned-at-obrizum commented Jun 14, 2023

Hey,
Neovim users usually have a hard time setting up formatting for their needs,
and some of us are not using community standard formatting..
Is it possible to add Better Clojure Formatting from Tonsky as one of the styles that can be applied?
https://tonsky.me/blog/clojurefmt/
Thanks.

@kkinnear
Copy link
Owner

An interesting question about the Tonsky's Better Clojure Formatting. Near as I can tell, his rules are what I call "indent only" kinds of rules. That is to say that I don't think his rules every cause the addition of another line. In part because there is no concept of "width" in Tonky's rules, so there would never be a reason to move something onto another line. Or, presumably, move something that was on a subsequent line up to a previous line.

I don't have any examples of the TCBF (Tonsky's Better Clojure Formatting) applied to complex situations. If I take his simple examples off of the web-page you linked me to above, I can exactly reproduce them with the standard :indent-only style. For example:

; i301 is basically the examples off of the TBCF website:

(print i301)
(when something
  body)

(defn f [x]
  body)

(defn f
  [x]
  body)

(defn many-args [a b c
                 d e f]
  body)

(defn multi-arity
  ([x]
   body)
  ([x y]
   body))

(let [x 1
      y 2]
  body)

[1 2 3
 4 5 6]

{:key-1 v1
 :key-2 v2}

#{a b c
  d e f}
 
; If I use i301 as the input to zprint with :indent-only

(zprint-file-str i301 "" {:style :indent-only})
"(when something\n  body)\n\n(defn f [x]\n  body)\n\n(defn f\n  [x]\n  body)\n\n(defn many-args [a b c\n                 d e f]\n  body)\n\n(defn multi-arity\n  ([x]\n   body)\n  ([x y]\n   body))\n\n(let [x 1\n      y 2]\n  body)\n\n[1 2 3\n 4 5 6]\n\n{:key-1 v1\n :key-2 v2}\n\n#{a b c\n  d e f}\n"
zprint.core=> (print *1)
(when something
  body)

(defn f [x]
  body)

(defn f
  [x]
  body)

(defn many-args [a b c
                 d e f]
  body)

(defn multi-arity
  ([x]
   body)
  ([x y]
   body))

(let [x 1
      y 2]
  body)

[1 2 3
 4 5 6]

{:key-1 v1
 :key-2 v2}

#{a b c
  d e f}

They really are the same:

zprint.core=> (zprint-file-str i301 "" {:style :indent-only})
"(when something\n  body)\n\n(defn f [x]\n  body)\n\n(defn f\n  [x]\n  body)\n\n(defn many-args [a b c\n                 d e f]\n  body)\n\n(defn multi-arity\n  ([x]\n   body)\n  ([x y]\n   body))\n\n(let [x 1\n      y 2]\n  body)\n\n[1 2 3\n 4 5 6]\n\n{:key-1 v1\n :key-2 v2}\n\n#{a b c\n  d e f}\n"
zprint.core=> (= i301 *1)
true

Will the zprint :indent-only style also reproduce what the TBCF style mandates in more complex situations? Hard to say, but it is entirely possible that it will. The zprint :indent-only style does not use the :fn-map and does not change formatting based on the function or macro being formatted. Which is what TBCF mandates. It also means that the :indent-only style will not wake up and do something odd with some function because it doesn't know about different functions.

I just reviewed zprint's :indent-only documentation, and I noticed that I used mostly the examples from the TBCF page there. I also remember that there was some discussion about aligning things that were already aligned, which zprint does using :style :indent-only. I don't remember where this discussion occurred, but I do remember reading it. You can see it in operation below:

zprint.core=> (print i301a)
(or (condition-a)
  (condition-b))
nil
zprint.core=> (czprint i301a {:parse-string? true :style :indent-only})
(or (condition-a)
  (condition-b))
nil
zprint.core=> (print i301b)
(or (condition-a)
    (condition-b))
nil
zprint.core=> (czprint i301b {:parse-string? true :style :indent-only})
(or (condition-a)
    (condition-b))
nil
zprint.core=> (print i301c)
(or (condition-a)
    (condition-b)
       (condition-c))
nil
zprint.core=> (czprint i301c {:parse-string? true :style :indent-only})
(or (condition-a)
    (condition-b)
    (condition-c))
nil
zprint.core=> (print i301d)
(or (condition-a)
      (condition-b)
    (condition-c))
nil
zprint.core=> (czprint i301d {:parse-string? true :style :indent-only})
(or (condition-a)
  (condition-b)
  (condition-c))

If you don't like the way that :style :indent-only detects the alignment of the third element in a list relative to the second element of a list and replicates that alignment for the rest of the list, you can disable it like this:

zprint.core=> (czprint i301c {:parse-string? true :style :indent-only})
(or (condition-a)
    (condition-b)
    (condition-c))
nil
zprint.core=> (czprint i301c {:parse-string? true :style :indent-only :list {:indent-only-style :none}})
(or (condition-a)
  (condition-b)
  (condition-c))

I hope that :style :indent-only meets your needs. Let me know how it goes for you if you can, as I would like to know your response to whether or not this meets your needs. Thanks!

@ned-at-obrizum
Copy link
Author

Hey,
Thanks for taking an interest in this topic,
I will modify my .zprintrc by following your instructions and see how it goes, it really looks promising!!
After I test it out I will let you know!
Thanks for the whole explanation, it really helped me.
If it is okey I will leave this Issue open and let you know how it goes. :D

@ned-at-obrizum
Copy link
Author

Hey,
Quick update.
I added :indent-only to the :style list
My .zprintrc looks like this now
{:style [:indent-only :justified :multi-lhs-hang]}
But it looks like :indent-only and :justified are having a problem working together, since I added :indent-only I don't get keys and value vertically aligned on left and right side
Do you have any ideas how to make them work together?

@kkinnear
Copy link
Owner

Yes, :indent-only is very much indent-ONLY. It pretty much does the basic two rules from the TBCF web-page, with a little bit of cleverness for the :indent-only-style. Which didn't include anything about justifying pairs. The :indent-only code doesn't even know about pairs, really, since it doesn't recognize any functions at all. It doesn't know where things are supposed to be pairs or not.

Now, I have an idea of how to get you most of what you want, but first I need to know from you what you want to see justified. I'm guessing that you want to have let bindings justified. I don't know what else might be important to you. Do you want map keys and values justified? Are there other places where justification is important to you?

I presume that you have noticed that justification isn't always chosen as the "best" approach, in that there are plenty of reasons that justification just doesn't look better so it is skipped. There are lots of things you (or I) can tune to make justification happen more frequently, though they don't necessarily make the output actually look all that much better.

Anyway, let me know where you particularly want justification, and I'll see what I can do to make that work for you along with something that works mostly like :indent-only.

@ned-at-obrizum
Copy link
Author

Great, what I am interested in is justification of let, map, cond, case. (If it's easy to set it up on all binding then great)
If justification is needed in more cases then this I will set it up myself, I just need to see how that is done and I will continue modifying if necessary, and eventually send you the conf so you can share with someone who needs it.

The thing is that I support your opinion on this, I also don't find justification very useful in some cases but still If I don't set it up then I will not be able to use my neovim for job related projects because that is one of the rules set by engineers in my company.
I wouldn't like to abandon neovim for doom emacs only because formatting.

So basically what I need is TBCF + justification on some bindings.

@kkinnear
Copy link
Owner

kkinnear commented Jun 17, 2023

I have something for you to try. In order to get justification, this approach does not use :indent-only. Instead I've attempted to configure the classic zprint algorithm to mimic the TBCF approach. It seems to work pretty well, but there are some caveats that will require some explanation and background. So please bear with me here.

First, let me draw a distinction between indenting tools and full-on formatting tools. An indenting tool essentially leaves everything on the line that it originally appeared on, and doesn't add or delete line endings. It attempts to indent everything on each line to its appropriate place on the line. A full-on formatting tool basically ignores all white space, and formats the file based on its internal rules.

Why do I make this distinction?

Because a full-on formatting tool will produce exactly the same output given any semantically identical input. That is, you can add or delete whitespace and newlines anywhere in the input, and the full-on formatting tool will always produce the same output. An indenting tool will not, of course, because it only adjusts where things are on their own lines. This is not to say that either is better or worse, but they are certainly different.

To the best of my knowledge, the TBCF formatting approach is an indenting approach. I do not believe that it says anything about what lines to place things on. Note these examples:

(defn f [x]
  body)
  
(defn f
  [x]
  body)
  
(defn many-args [a b c
                 d e f]
  body)

It shows how the TBCF rules will format these lines, but it doesn't say "put the args on the same line as the defn" or "put the args on a line below the defn".

Thus, the TBCF rules don't specify exactly what the file is to look like -- it specifies exactly what the indents should look like given whatever file you have.

As it happens, classic zprint (not using :respect-nl) is a full-on formatting tool. It will always produce the same output given any semantically identical input. But we aren't going to use it that way, since that isn't what you need.

Now when I configure zprint with :respect-nl, it will never remove any of your line endings. It will, however, add line endings where it thinks you need them. If we also set the width: :width 500, then it won't add line endings because your lines are too long. In order to get things like case to justify, I have to configure case to be :arg1-pair-body. That will cause any case that doesn't fit on one line to have the first element of the case on the same line as case, and all of the other elements paired up (and in your case justified) below them, starting on the next line. Probably this isn't a problem, since you probably do that already. Some examples:

; This will will work

(case stuff
  :foo "bar"
  :baz "bother")

; This will work because of :respect-nl

(case
   stuff
  :foo "bar"
  :baz "bother")

; You can't do this - but you probably didn't want to anyway
(case stuff :foo "bar"
                   :baz "bother")

The point here is that your local coding standards probably indicate what lines you should place things on, and probably they don't conflict with what zprint is going to do -- particularly since with :respect-nl all newlines are respected. So you probably aren't going to have any problems as long as you create files which meet your local standards of what lines things should be on. But zprint isn't going to make that happen for you -- you have to do that yourself.

So, here is an example of what I cooked up to meet your needs:

zprint.core=> (zprint-file-str i301e "x" {:style [:respect-nl :justified :drop-fm] :style-map {:drop-fm {:fn-map nil}} :list {:wrap? true :option-fn (fn ([] "TBCF") ([opts n exprs] (when-not (or (keyword? (first exprs)) (symbol? (first exprs))) {:list {:indent 1}})))} :fn-map {"let" :binding "cond" :pair-fn "case" :arg1-pair-body} :width 500})
...
zprint.core=> (print *1)
(when something
  body)

(defn f [x]
  body)

(defn f
  [x]
  body)

(defn many-args
  [a b c
   d e f]
  body)

(defn multi-arity
  ([x]
   body)
  ([x y]
   body))

(let [x      1
      y-long 2]
  body)

[1 2 3
 4 5 6]

{:key-1      v1,
 :key-2-long v2}

#{a b c
  d e f}

(1 2 3
 4 5 6 7
 8 9)

'(1 2 3
  4 5 6 7
  8 9)


(or (condition-a)
  (condition-b)
  (condition-c))

(defn compare-ordered-keys
  "Do a key comparison that places ordered keys first or last, based on
  where they fall with respect to the value of a divider -- :|."
  [key-value divider-value zdotdotdot x y]
  (cond (and (key-value x) (key-value y)) (compare (key-value x) (key-value y))
        (key-value x)    (if (< (key-value x) divider-value) -1 +1)
        (key-value y)    (if (< (key-value y) divider-value) +1 -1)
        (= zdotdotdot x) +1
        (= zdotdotdot y) -1
        :else            (compare-keys x y)))

(let [l-str     "#<"
      r-str     ">"
      indent    (count l-str)
      l-str-vec [[l-str (zcolor-map options l-str) :left]]
      r-str-vec (rstr-vec options ind zloc r-str)
      type-str  (case zloc-type
                  :future  "Future@"
                  :promise "Promise@"
                  :delay   "Delay@"
                  :agent   "Agent@")]
  (stuff))

(defn integrate-next-inner
  "If the value of :next-inner is a map, then config-and-validate it. If
  the value of :next-inner is a vector of maps, then config-and-validate
  each of the maps in turn."
  [options]
  (dbg-pr options "integrate-next-inner:")
  (let [next-inner (:next-inner options :unset)]
    (cond (map? next-inner)     (first
                                  (zprint.config/config-and-validate
                                    "next-inner:"
                                    nil
                                    (dissoc options :next-inner)
                                    next-inner
                                    nil ; validate?
                                  ))
          (vector? next-inner)  (reduce
                                  #(first
                                     (zprint.config/config-and-validate
                                       "next-inner-vector"
                                       nil
                                       %1
                                       %2
                                       nil))
                                  (dissoc options :next-inner)
                                  next-inner)
          (= next-inner :unset) options
          :else                 options)))

The examples show how it works.

There is one place where I want to point something out. In compare-ordered-keys, on the first line of the cond, you will notice that the justification doesn't actually happen for the first line, but it does for the rest of the lines. That is because of the :max-variance setting for the justification. Justification is configured by default to allow up to two lines that exceed the :max-variance to not be justified, allowing the others to be justified with a narrower width. You can configure this, as shown below (the justification configuration is at the very end of the options map).

zprint.core=> (czprint i301f {:parse-string? true :style [:respect-nl :justified :drop-fm] :style-map {:drop-fm {:fn-map nil}} :list {:wrap? true :option-fn (fn ([] "TBCF") ([opts n exprs] (when-not (or (keyword? (first exprs)) (symbol? (first exprs))) {:list {:indent 1}})))} :fn-map {"let" :binding "cond" :pair-fn "case" :arg1-pair-body} :width 500})
(defn compare-ordered-keys
  "Do a key comparison that places ordered keys first or last, based on
  where they fall with respect to the value of a divider -- :|."
  [key-value divider-value zdotdotdot x y]
  (cond (and (key-value x) (key-value y)) (compare (key-value x) (key-value y))
        (key-value x)    (if (< (key-value x) divider-value) -1 +1)
        (key-value y)    (if (< (key-value y) divider-value) +1 -1)
        (= zdotdotdot x) +1
        (= zdotdotdot y) -1
        :else            (compare-keys x y)))
nil
zprint.core=> (czprint i301f {:parse-string? true :style [:respect-nl :justified :drop-fm] :style-map {:drop-fm {:fn-map nil}} :list {:wrap? true :option-fn (fn ([] "TBCF") ([opts n exprs] (when-not (or (keyword? (first exprs)) (symbol? (first exprs))) {:list {:indent 1}})))} :fn-map {"let" :binding "cond" :pair-fn "case" :arg1-pair-body} :width 500 :pair {:justify {:max-variance 1000}}})
(defn compare-ordered-keys
  "Do a key comparison that places ordered keys first or last, based on
  where they fall with respect to the value of a divider -- :|."
  [key-value divider-value zdotdotdot x y]
  (cond (and (key-value x) (key-value y)) (compare (key-value x) (key-value y))
        (key-value x)                     (if (< (key-value x) divider-value) -1 +1)
        (key-value y)                     (if (< (key-value y) divider-value) +1 -1)
        (= zdotdotdot x)                  +1
        (= zdotdotdot y)                  -1
        :else                             (compare-keys x y)))

You can also see why (at the suggestion of a user) I implemented the variance based justification. I don't know how this will play with your local standards requiring justification.

Let me explain the entire option map I'm using:

(zprint-file-str i301e
                 "x"
                 {:style [:respect-nl :justified :drop-fm],
                  :style-map {:drop-fm {:fn-map nil}},
                  :list {:wrap? true,
                         :option-fn (fn
                                      ([] "TBCF")
                                      ([opts n exprs]
                                       (when-not (or (keyword? (first exprs))
                                                     (symbol? (first exprs)))
                                         {:list {:indent 1}})))},
                  :fn-map
                    {"let" :binding, "cond" :pair-fn, "case" :arg1-pair-body},
                  :width 500})

First, there are approximately 122 things defined in the :fn-map, and we need to get rid of them so that zprint doesn't know about all of those functions -- and then just add back in the ones you want. As it happens, zprint doesn't have any convenient way to get rid of the entire :fn-map -- something that I will be fixing in a near-future release. But in order to make this work for you, I've concocted a real hack to wipe out the :fn-map and then replace it with just what you need. That is the definition of the style :drop-fm, and the use of that style. Together they will remove the entire :fn-map -- and, incidentally, leave zprint unable to operate. But since we put back a small :fn-map with just "let", "cond" and "case", we fix that problem.

Next, in :list, we set :wrap? true which will cause multiple elements of a list to stay on the same line -- unless you have put them on separate lines yourself. In other words, zprint will not add newlines to lists. Also in :list, there is an :option-fn which runs whenever a list is encountered. This function checks the first element of the list, and if it is a symbol or a keyword, it uses the configured indent (which is 2). If it is not a symbol or a keyword, it will use 1 for the indent (which is what TBCF specifies and is a good idea anyway).

Next, we load the :fn-map with your three functions. You also said that you wanted justification for "map", but I'm guessing you meant "maps" -- which are not functions but instead have their own configuration element -- :map. If you use :style :justified, you get justification for maps too. While we are on that subject, note that :binding, :pair, and :map are all things with pairs and all have their own configuration sections in the option map. Each one, for instance, has its own :justify key and each one has its own :max-variance within its :justify configuration. So if you mess about with the :max-variance, note that changing it for one thing won't change it for other things. Also note that cond and case pairs are configured with :pair, let bindings are configured with :binding, and map elements are configured with :map.

If you want to have other functions with pairs (or bindings) be justified, you can add them to the :fn-map section of the option map. Be sure to look to see what they are configured for by default before you do that. You can look at GitHub in the sources for config.cljc, and toward the front you can see all of the default configuration for the :fn-map.

That's all I can think of right now. I hope that this gets you some of the way toward where you want to go.

Let me know if this doesn't work, and I'll see what I can do. I think it is close.

Thanks, by the way, for asking. I enjoy trying to figure out how to get zprint to meet people's needs, and this is certainly an interesting challenge.

@ned-at-obrizum
Copy link
Author

Thank you for this detailed explanation, it will help a lot if I need to change anything.
This looks like it will work for me.
I really like it how zprint can be custom tailored per certain needs!

If I stumble on some case where it needs tweaking I will let you know.
I am sure that someone else will want to use this for his/her neovim too.
Maybe it's not a bad idea to put it in the documentation as example of how to integrate it with TBCF.
And I will see to recommend zprint to other people that have formatting/indenting issues.
Thanks a lot!

kkinnear added a commit that referenced this issue Sep 13, 2023
@radovanne
Copy link

radovanne commented Feb 28, 2024

Hey Kim,

Sorry for the delay in getting back to you. I've made some tweaks to my .zprintrc file, mainly removing commas during formatting as they're considered bad practice in Clojure code. The style closely resembles Tonsky's Better Clojure Formatting (BCF), with just a few minor adjustments needed for proper indentation ( If you think that anything needs to be added to this zprint config please tell me, to me it looks like a close match ).

You can find the modified .zprintrc file here: https://github.com/radovanne3/neovim/blob/27e802c50bf66b7f98cbf28c093204f139db201f/.zprintrc

Since null-ls isn't maintained anymore, I've adapted my setup to work with none-ls instead. Here's how I've configured it: https://github.com/radovanne3/neovim/blob/27e802c50bf66b7f98cbf28c093204f139db201f/lua/verde/none-ls.lua

To avoid conflicts with the default zprint program on Mac, I've renamed your zprint executable to zprintm.

Given that the BCF style is, I guess, being used, perhaps it could be nice if you included as one of the predefined styles. This way, it would be easier for everyone to adopt.

Because of not being able to use BCF style in neovim before I used Emacs, but now I don't need anymore.
Maybe there are more people in same situation.

Either way, thank you for support!

( Before replying here I replied to one closed issue of mine, that was a mistake. )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants