Calva doesn't respect .cljfmt.edn indent rules with namespaced function names

[tlaundal]

I've been having some issues with custom cljfmt indent rules in Calva. It seems that Calva doesn't resolve namespaces on the indent rules, so the it isn't picked up when formatting the relevant forms.

I've made a reproduction repo which reproduces the issue for me in a fresh vscode profile with just Calva installed.

Calva and cljfmt from commandline agrees when I spell out the full namespace of the macro, as demonstrated in the .cljfmt.edn file:

{:extra-indents {repro.macro/example-macro [[:block 2]]}

 :test-code [;; cljfmt and Calva agree on how to format this:
             (repro.macro/example-macro :first-arg second-arg
               (some long body))]}

But when the macro is used with a :require [... :as ...], Calva and cljfmt gives different results, as shown in the repro namespace:

(ns repro.repro
  (:require
   [repro.macro :as macro]))

;; Calva and cljfmt fight for how this should be formatted
(macro/example-macro :first-arg second-arg
  (some long body))

Adding :alias-map {macro repro.macro} to .cljfmt.edn doesn't help, neither does disabling the new indent engine in Calva or manually specifying the path to the cljfmt config.

If I change the rule to just :extra-indents {example-macro [[:block 2]]}, it works.

[PEZ]

Hello, thanks for reporting!

I think part of the problem here is that Calva only formats the form enclosing the cursor. So only the macro invocation in the above example. That means that the cljfmt in Calva never sees the ns form and can't figure the alias out.

I'm not sure what the fix would be, or even if this is the whole story, but anyway.

[tlaundal]

Thanks for the prompt reply, Pez!

Even though Calva doesn't read the ns form, I would expect it to support the :alias-map - but I couldn't get that to work either. Am I doing something wrong with :alias-map {macro repro.macro}?

Would it be possible to parse the ns form into an alias-map and pass that to the formatter along with the user-defined alias-map?

I see alias-map is supported in pez-cljfmt, but I couldn't find it in cursor-doc (which I think is the old indent engine?) Nonetheless I couldn't get it to work even when disabling calva.fmt.newIndentEngine.

[PEZ]

Let's calibrate a bit:

1. Auto-indentation happens when you hit enter to get a new line. This is the job for the indent engine. The (new) indent engine doesn't use cljfmt, but reads the cljfmt config to try to do “what cljfmt would have done”. It doesn't support the :alias-map setting, but probably it should. It doesn't do any attempt to understand aliasing, even though it technically has access to the ns form. Probably it should. Disabling it will fall back on the legacy behaviour of using cljfmt for the indentation decision. It would help with the issue if the cljfmt integration would handle alises. But it wouldn't help with formatting. (Indeed it's in the cursor-doc section of the code base, as it leverages Calva structural editing facilities.)
2. Formatting. The commands for formatting when Calva is configured as the formatter is done by the formatter. As well as a lot of auto-formatting happening as you structurally edit the document. The formatter uses cljfmt as a ClojureScript library to do its work. Most often just the portion of the text around the cursor is sent to the formatter. I haven't yet checked wether :alias-map is being sent along properly, or how cljfmt is supposed to use it. So there may be that we can fix something around this.

pez-cljfmt is a decoy. 😄 It's an old fork of cljfmt that is only used when you use the Format and Align command.

[tlaundal]

Thanks for the details. It seems we have a few different sub-issues which can be worked on. From least to most effort I am thinking:

1. Make sure :alias-map is sent along correctly to cljfmt for formatting
2. Implement support for :alias-map in the new (and old?) indentation engine
3. Implement a mechanism for parsing the ns-form into an :alias-map, so it can be used by both formatting and indentation engine

I haven't been able to get :alias-map to work with any of the Calva/VSCode format commands or when using the indent engine (inserting newline, pressing tab), but I also haven't been able to get it to work with cljfmt, so I'll open an issue there as well just to check if I'm doing something wrong.

Also, regarding cljfmt as a cljs library, this issue from cljfmt might be relevant: weavejester/cljfmt#200

[PEZ]

Makes sense. I now checked what happens with :alias-map in Calva. TL;DR: Nothing. Calva passes on whatever is in the config to cljfmt/reformat-string. Calva may touch some config on keys that it cares about, but since no attempts have been made for treating :alias-map in any special way, that value will reach cljfmt unadulterated.

That + your research indicate that this may just be untapped potential in cljfmt as a ClojureScript library, as @lread has since quite a while fixed the underlying reasons.

3. Implement a mechanism for parsing the ns-form into an :alias-map

This could also be solved by including the ns-form when handing over the text to cljfmt and then stripping it away again when handling the results of the formatting. I think that is easier than parsing out the aliases. And it would also centralize this parsing to cljfmt, avoiding confusion in bug incompatibilities and such. (I'm not saying I have thought this through and know which way is best, just thinking out loud.)

Wanna have a go at 3., @tlaundal? If so we should speak to @pbwolf who is making some changes to the formatter integration in a separate PR. Not that we need to block things up on each other, but at least have a chat and know more about what is going on.

[tlaundal]

So after WeaveJester pointed me in the right direction for using :alias-map in weavejester/cljfmt#363, I actually have it working reliably in Calva now for both indent and formatting (even Format and Align) - which I thought was surprising. The backside of :alias-map of course, is that it's not helpful for macros used in the same ns where they are defined.

3. Implement a mechanism for parsing the ns-form into an :alias-map

This could also be solved by including the ns-form when handing over the text to cljfmt and then stripping it away again when handling the results of the formatting. I think that is easier than parsing out the aliases. And it would also centralize this parsing to cljfmt, avoiding confusion in bug incompatibilities and such. (I'm not saying I have thought this through and know which way is best, just thinking out loud.)

Yes, I was also thinking about a solution like this. I guess the trade-off is between the complexity of stripping away the ns from the parsed output versus parsing it ourselves - unless it's easy to hook into cljfmt for this.

Wanna have a go at 3., @tlaundal? If so we should speak to @pbwolf who is making some changes to the formatter integration in a separate PR. Not that we need to block things up on each other, but at least have a chat and know more about what is going on.

Yes, I think it would be fun to have a go at this. Since I'm completely new to the Calva codebase I would love some pointers for where and how to start implementing this.

@pbwolf could you chime in with how your PR would affect this?

[PEZ]

Cool!

It starts on the wiki under https://github.com/BetterThanTomorrow/calva/wiki/How-to-Hack-on-Calva

• The integration with cljfmt is in the fun (i.e. ClojureScript) part of the code base:
  https://github.com/BetterThanTomorrow/calva/blob/published/src/cljs-lib/src/calva/fmt/formatter.cljs
• The immediate TypeScript side is here: https://github.com/BetterThanTomorrow/calva/blob/published/src/calva-fmt/src/format.ts

I don't right now recall if we always get :all-text in from the editor to formatter.cljs. If we do, then you could extract the ns form from there. If we don't always have :all-text (and even if we do) we could pass the nsForm in from Calva's structural editor when we call the formatter code. This is in the TypeScript part of the code. You can search the codebase for nsForm to see how we pick that out. A thing with using nsForm is that it is not always the ns form of the document. It is the form that Calva will use for evaluations, which can be an in-ns form in some comment and such things. Maybe we should create a utility for considering only ns forms. I am working on a PR which would benefit from that too.

If you are on Clojurians Slack, you can always say hello in the #calva channel. I'd be super happy to screen share and show you around the codebase a bit.

Very surprising that :alias-map even worked with the alignment command. Maybe I kept the fork updated for long enough before I gave up on that strategy...