Fork me on GitHub
#clojure
<
2022-07-11
>
practicalli-johnny08:07:26

Anyone successfully using Clojure Setup with Reviewdog (or similar GitHub action) to add comments to PRs when tests, linting and code formatting checks (cljstyle or cljfmt) return errors / warnings ? I can look in the Action logs, but its far faster to have the specific in the PR conversation https://github.com/nnichols/clojure-lint-action works very well for just clj-kondo but my own efforts to add other things have been unsuccessful so far. Anyone had any success in this area?

nnichols13:07:57

I had to do three things to get in-PR comments: • Set the ReviewDog reporter flag to github-pr-review • Change the clj-kondo output to {:pattern "{{filename}}:{{row}}:{{col}}: {{message}}"} • Provide a PAT via an environment variable to write the comments on the PR REVIEWDOG_GITHUB_API_TOKEN For style/formatting, I took a different approach with Wall Brew. Since sharing secrets across repos is easier with an org, I created a special bot account (also to act as the billing/owning user). I use its token to auto-format each PR and then automatically commit that change. Here's an example if you're curious: https://github.com/Wall-Brew-Co/brewtility/blob/master/.github/workflows/format.yml To integrate a formatter with reviewdog, you'd probably have to modify either the input formatter of reviewdog or the output formatter of cljstyle/cljfmt

chef_kiss 1
👍 1
Martynas Maciulevičius14:07:49

For me Clojure's custom formatting rules were always a problem when working with people that don't use Emacs/Spacemacs. Change my mind. For instance we have this (in Spacemacs):

(inc
 1
 2)
(inc 1
     2
     3)
And then we have this:
(let []
  1
  2)
And also this:
(is
 =
 1
 2)
inc is a function, let and is (taken from clojure.test) are macros. But even macros are formatted differently.

Ben Sless14:07:52

Counterpoint: for me, usage of intellij has always been a problem for proper Clojure formatting standards

Ben Sless14:07:24

As is evident by your example, indentation rules in a proper editor are way more sophisticated (and better)

p-himik14:07:32

When it comes to indentation, at least Cursive is rather flexible. Each symbol, when used in the call position, can have its own indentation. So I can indent let in one way and, say, when-let in a different one. Not that I'd do that.

Martynas Maciulevičius14:07:34

I currently configure nvim with conjure and I don't get where to configure the inc to use one space and not two. And it still uses cljfmt (I hope) to format everything. So there could be one tool that is supposed to do formatting and we could use it.

lispyclouds14:07:37

I mostly get around the issue on clojure teams by checking in the .zprintrc as we all use https://github.com/kkinnear/zprint with at least 4 different editors

lispyclouds14:07:12

since nvim is mentioned, i use it too and format using :% ! zprint '{:search-config? true}' from anywhere in the project

Martynas Maciulevičius14:07:04

I think that on IntelliJ you should get this by default (probably, I'm not sure):

(inc 1
  2
  3)
It would be problematic for me. Does Spacemacs/Emacs accept .zprintrc? Because I'm very sure it doesn't accept .editorconfig (for clojure, it works for others). Which editors did you use? Regarding nvim -- is there a way to format it just as Spacemacs does it? I think that Spacemacs is a great editor and the style is good (I used it a lot) but the bindings are somehow hardcoded and that's why it's said that it's "the holy grail of Clojure style" which... for that reason nobody can mimic reliably. I found this guide: https://clojure-lsp.io/clients/#nvim. But I wouldn't call it a guide as it requires to replace the whole nvim folder. I think that .zprintrc could probably work for nvim but then how would we provide .zprintrc to Emacs to use if it doesn't care about .editorconfig? Then that means that the .zprintrc file must be the source of truth for formatting and not the Clojure's style guide which says that it's Emacs that is the "way".

lispyclouds14:07:45

Well being not an Emacs user myself but I'd suspect the others are using a similar way of piping the buffer/region to zprint i guess

lispyclouds14:07:05

theres M-x shell-command-on-region in emacs

nbardiuk14:07:07

for clojure-lsp you don't need to replace nvim folder, you just need a working lsp setup. Then project can have its own format rules https://clojure-lsp.io/settings/#project in .lsp/config.edn

p-himik14:07:46

Assuming you mean Cursive by "on IntelliJ", what you describe is not the default behavior. Hard to check right now, but IIRC the default is putting function arguments right underneath each other if the first one is on the same line as the function itself. The 2-space indentation, as in your case, is used for when the first argument is on the next line.

lispyclouds14:07:51

> not the Clojure's style guide which says that it's Emacs that is the "way" you can configure zprint with {:style :community} to follow the clj style guide

lispyclouds14:07:41

but yeah our team has their opinions lets say and (finally) settled on a zprintrc 😅

Martynas Maciulevičius14:07:13

I don't remember how IntelliJ formats it and I don't want to bash on IntelliJ. They also have their own quirks how they lock people in with the formatters but it's not the point that I'm trying to make. I like that golang has gofmt tool which is everywhere the same. And our alternative in Clojure is... Emacs as the source of truth which can't be used for people that don't use it. I think this is not good. Why not then have a single repository with the style configs for each tool then. Maybe .zprintrc community config is one way to do it. I'll try it. Regarding nvim and clojure-lsp - why not propose a minimum possible package list. I installed clojure-lsp-bin and coc but I don't know if anything changed. It still probably uses Conjure to format clj code. So I don't know whether I completed the guide. So probably the most understandable and non-magic way would be to add a file-save callback that runs zprint and formats the file.

lispyclouds14:07:30

yeah being on nvim/vim for a while now, formatting means pipe to a cli thing in my mind now 😛 not a editor thing

Joshua Suskalo15:07:16

I find cljfmt very limiting both in the indentation styles it allows and the fact that it doesn't allow library authors to distribute format rules for macros they write. I much prefer CIDER's auto-indenter and the support for :style/indent metadata on functions and macros.

Martynas Maciulevičius15:07:18

Yes but CIDER means forced use of Emacs :thinking_face:

lispyclouds15:07:20

I wish every tool supported the metadata 😕 its the most elegant way

Martynas Maciulevičius15:07:12

Does zprint support metadata formatting?

Joshua Suskalo15:07:23

Yeah, I don't like that cider's indentation is not standard. I was working on using clj-kondo's analysis tool to start working on a clojure indenter that would work from the commandline like cljfmt and support cider's style, but unfortunately I haven't had the time to continue its development.

Joshua Suskalo15:07:06

But I do consider the ability to distribute indentation rules with libraries absolutely critical to any formatting tool.

Joshua Suskalo15:07:52

Without it you get absolutely terrible indentation of custom macros, like the ones in #farolero which have complex indentation rules to follow CL's standardized formatting.

lispyclouds15:07:39

> Does zprint support metadata formatting? Dont think so. But its config is fairly flexible, less than metadata of course.

Martynas Maciulevičius15:07:48

I think that as it supports the config in EDN format it can be extended to support the macro formatting. Not by-default and there would be a lot of work but at least it has a config file that could specify the rules :thinking_face:

Joshua Suskalo15:07:35

Right, but that's part of what I'm talking about. It doesn't really matter if it's specified by metadata, or an edn file, or whatever else. cljfmt supports edn files to specify indent specs, that's fine. The problem is that cljfmt specifically doesn't support distributing indentation rules with libraries.

Martynas Maciulevičius19:07:57

@U7ERLH6JX Consider adding this to your .vimrc: autocmd FileType clojure set equalprg=zprint\ \'{:search-config?\ true}\' This allows to use = for your formatting without piping it on write

lispyclouds19:07:26

@U028ART884X yep, have something similar with a leader mapping. gotten too used to the leader 😛

lispyclouds19:07:39

but the still somehow type the command, guess i jump around machines too much, my confing's not everywhere

Martynas Maciulevičius19:07:19

But now I see that zprint does some brutal comma additions and line breaks. So it's not over yet 😄 But oh man... it's so much faster than Spacemacs.... Even if I know that it will do multiple config file lookups until it'll find the root one...

lispyclouds19:07:39

You can set :map {:comma? false}. Line breaks need to be tuned a bit, one of :flow or arg1-force-nl settings, its quite flexible

💯 1
Martynas Maciulevičius19:07:05

Why is there so much to configure...... wow. Also it doesn't work for a single line.

lispyclouds20:07:16

well when you dont use the pre configured style and go down the zprint rabbit hole 😅 > Also it doesn't work for a single line. as in formatting a single line of clojure?

Martynas Maciulevičius20:07:55

Yes. If I try to format a single line of code then it fails with an error. It "just works" in CIDER though. And works really well. But the performance though... it has some performance. Edit: well... 1k LOC clj file is not that fast. But nvim still has the performance.

lispyclouds20:07:17

This seems to work for me

$ echo '(defn foo [a b] (let [x 10] (+ a b x)))' | zprint '{:search-config? true}'
(defn foo
  [a b]
  (let [x 10]
    (+ a b x)))

lispyclouds20:07:12

also tried in nvim, works there too

Joshua Suskalo20:07:49

I think the problem here is putting point over just the arg vector there and trying to format just that

Martynas Maciulevičius20:07:53

When I press == in nvim then it sends this kind of input:

echo '  (let [port (or port default-server-port)' | zprint
And that can't not fail. It would be handleable if it would send the whole file instead and also send the range. But that's quite a bit to handle via CLI and vimscript. I don't even know how to do it 😄 It would be too good for me. I don't think I need it.

lispyclouds20:07:36

yeah i work around it with visual-line selecting the whole top level form like def or defn etc and send that to zprint with a range. lets see if i can figure out a line range, bit tricky

Martynas Maciulevičius20:07:58

No it's fine. I think I'll be selecting a paragraph with =ap or I'll make a motion that would select whole top-level form.

lispyclouds20:07:28

yeah and if youre using tressitter you can make the motion AST aware too

lispyclouds20:07:11

https://github.com/vim-scripts/vis seems to sort of do it, the substitution back is a bit weird

Martynas Maciulevičius02:07:32

You know... IMO zprint underdeliver. I know it's very hard to make it right but it's not the same experience. It does all of the good formatting things but at the same time it's not consistent with what Emacs does because you must manually find the level of compatibility that you want. So I wouldn't call zprint something that works out of the box. It has the right idea but :community didn't mean "as in Emacs". It's different because it lacks some kind of less strict heuristics. I think that Emacs's Clojure rules are too much to handle. And that could mean that they should have been separated from the start and not hardcoded. Probably that means that zprint is a good middle-ground but it's not easy to tell everybody that the standard formatting tool should be something else (not even zprint or any of the current ones). But then why not take part of Emacs-lisp code which formats the code and why not compile it into a standalone formatter binary....

lispyclouds07:07:11

we need to i guess agree on a set of formatting rules and have tools like go fmt or python black which have no config options. i would love that!

Martynas Maciulevičius07:07:39

I would call it "reusable source of truth". Because Emacs is a source of truth for Emacs but it's not reusable. Styleguide is also a code of truth but it's not executable as nobody wants to format the code by hand (actually I did it on 4clojure but back then I didn't care much and 2 spaces worked wonders). So IMO we don't need to agree on the rules because the rules are already agreed on. I think we have to agree on the rules+config or we have to use a reusable code chunk that can be executed from command line. And then if somebody wants to change the community code guidelines they have to make a PR instead of... I don't know what.

lispyclouds07:07:51

Agreed. https://github.com/bbatsov/clojure-style-guide is pretty much the source for me. Just wish for a thing that does this without any config options 🙏

Martynas Maciulevičius08:07:10

But it has txt files. And from my understanding txt files is not something one would execute or enforce via code. Also it doesn't say why this formatting is a thing (with your config it makes the spacing two spaces but if you remove 2 spaces from list config then it becomes this. I copied it from Spacemacs and zprint also formats this code like this when I have :community set and not 2 spaces):

(Thread/setDefaultUncaughtExceptionHandler
 (reify Thread$UncaughtExceptionHandler    ;; One space here for some reason.... magic
   (uncaughtException [_ thread ex]        ;; but two spaces in the implementation of a class
     )))

Joshua Suskalo13:07:50

The problem with having a gofmt or python black or rustfmt or similar is that we have structural macros. Those need customization in order to allow them to look good, and people will disagree at some point about how a given macro should be indented. So unless we want to go all-out and make blanket indentation rules to apply to all forms with no exceptions (which I think is a bad idea), I don't think we can really make a "no config formatter"

1
Joshua Suskalo13:07:36

Also there is a reason for the one space for that: any function will indent the list as a pure list, which makes all elements vertically aligned with the last one to appear on the first line. Since there's no arguments, it indents to match the function name. reify is a macro that has one special argument and "defn-style" indentation of each subsequent form, hence two spaces, which is the standard for general "this line needs indentation" in clojure.

Ben Sless13:07:18

What about an api for forms to describe how they should be indented via metadata?

Joshua Suskalo13:07:32

We do already have that, it's the :style/indent spec which is well-defined, but currently only implemented by CIDER.

Ben Sless14:07:41

So why not formalize it and make it ubiquitous standard?

Martynas Maciulevičius15:07:43

The style guide "is the standard". But it's unfortunate that text files don't work on IntelliJ or nvim. It worked on their machines when they coded it into Emacs though. I.e. clojure styleguide says it's a ubiquitous standard. So do we need another one? But then that text file is unusable and hard to mimic. So... thanks.

Ben Sless15:07:18

You have to account for the standard being written when Emacs was probably the main or most viable clojure ide

Joshua Suskalo15:07:39

@U028ART884X the standard being mentioned here from CIDER is not the same thing or even related to the style guide. The style guide instructs you on how to format general code in the general case. The CIDER indentation spec is a standard for defining how to indent exceptions to those rules. The two are orthogonal and in my opinion should be used together. Creating a formatting tool that implements the style guide is great, cljfmt can do that, but it is not enough because it doesn't also implement either the CIDER indentation spec or another method to distribute indentation rules for exceptions to the style guide.

Joshua Suskalo15:07:36

And critically, exceptions to the style guide are necessary, because syntax is extensible and there's no reason that clojure.core syntax should be held up as "special" when it comes to allowing custom indentation specs for specific forms.

Joshua Suskalo16:07:30

as e.g. let and defn indent differently from function calls, unless you adopt the relatively extreme measure of "everything uses defn indentation", which notably doesn't follow the style guide.

Martynas Maciulevičius16:07:32

Maybe I have done too much golang and that's why I think about one holy way of identation... :thinking_face: Uhh....

Joshua Suskalo16:07:02

"only one way to indent" makes perfect sense in a language with a fixed syntax. We don't have that though, we have syntax that is extensible to your domain, and we want our domain-specific code to look just as good as our general code. It's pretty easy to make "only one way to indent" for all the core stuff, and define a way to allow domain-specific code to indent in other ways that critically are consistent with the core indentation method, which is exactly what cider's indentation spec allows.

lispyclouds17:07:04

Adding to @U5NCUG8NR the fact that styling based more on semantics rather than syntax (clojure/lisps have only so much syntax 😆) necessitates that we can't have a fixed formatting rules.

Joshua Suskalo17:07:30

That's fair, although I'd personally call user-defined macros syntax rather than semantics, since you have to know about specific macros in order to know what of their arguments they evaluate and what they don't.

1
lispyclouds18:07:50

Also I'd wanna say even some data structures could have formatting styles like reitit routes or malli schemas :thinking_face:

Joshua Suskalo18:07:44

that'd be a little harder to auto-detect and a little more likely to just be a case where you deliberately ignore the auto-indenter.