jacobobryant / platypub

A publishing platform built with Biff
MIT License
65 stars 13 forks source link

Do something to enforce/encourage consistent code formatting #27

Closed jacobobryant closed 2 years ago

jacobobryant commented 2 years ago

So after writing clojure full-time and solo for the past 3.5 years, this will actually be the first project I've worked on with other people :). I've been using vim's defaults with a couple minor tweaks, however I'm not sure if all of vim's choices are mainstream? (e.g. 2-space argument indentation for function calls).

At a minimum perhaps it'd be helpful to have a style document, and whenever formatting differences come up in PRs, decide on a style and then mention it in the document? Perhaps include editor configurations? Or should we just add a command to the task script that formats the code with cljfmt and be done with it? Whatever we do, I'm not picky about formatting and am happy to change my editor config to match the mainstream.

For people who haven't been doing all their coding in a cave out in the wilderness, what do the projects you work on usually do?

jeffp42ker commented 2 years ago

I'm currently using Calva. I believe its default formatting rules are defined in clojure-lsp here: https://github.com/clojure-lsp/clojure-lsp/blob/master/.cljfmt.edn

{:remove-surrounding-whitespace? true
 :remove-trailing-whitespace? true
 :remove-consecutive-blank-lines? true
 :insert-missing-whitespace? true
 :align-associative? false
 :indents {#re "^(?!catch-kondo-errors).*" [[:block 0]]
           catch-kondo-errors [[:inner 0]]}
 :test-code
 (comment
   (:require
    [])
   (:require []
             [])
   (:clj
    [])
   (reg-event-fx
    :foo
    (fn [{db :db} _]))
   (reg-sub
    :foo
    (fn [db [_]])))}

We can define project formatting in a .lsp/config.edn file and check it in for project consistency. I don't yet understand the :indents value to know what to set to match the existing Platypub formatting style:

 (comment
   (:require
     [])
   (:require []
     [])
  )
jeffp42ker commented 2 years ago

This looks like a cljfmt config that matches the existing style of Biff and Platypub.

{:indentation?                    true
 :remove-surrounding-whitespace?  true
 :remove-trailing-whitespace?     true
 :remove-consecutive-blank-lines? true
 :insert-missing-whitespace?      true
 :align-associative?              false
 :indents                         {#"^\w" [[:inner 0]], #".*" [[:inner 1]]}}

For Calva, I've put it in a .cljfmt.edn file in the project's root dir and refer to it with the setting:

{"calva.fmt.configPath": ".cljfmt.edn"}

References: https://github.com/weavejester/cljfmt https://github.com/venantius/vim-cljfmt https://calva.io/formatting/

jacobobryant commented 2 years ago

I'll take a look at that!

jeffp42ker commented 2 years ago

The above cljfmt might match Tonsky's formatting from what I've seen so far.

This means I did need to manually align the and and or function arguments as they just wrapped and indented two spaces, which is mentioned as the one case where the simpler formatting doesn't look as expected.

mcb9

jacobobryant commented 2 years ago

I just tried out cljfmt and added a ./task format command to run it. I'm fine with the defaults, with the one exception being for submit-tx since that's such a common function and IMO it's worth it to make it more compact. So {:indents {submit-tx [[:inner 0]]}} can be used for config. (For the format command I just had it more-or-less pass it via the CLI: https://github.com/jacobobryant/platypub/blob/master/task#L33)

At some point we should probably have a "contributing" section in the README and mention the command + suggest adding it to your editor. For now I'll probably just run it every now and then myself if needed,