Setting options to customize cljfmt does not take effect

[laheadle]

Expected behavior

Setting the option to customize the behavior of cljfmt should take effect by customizing that behavior.

Actual behavior

Setting the option to customize the behavior of cljfmt has no effect.

Steps to reproduce the problem

I have set:

(setq cider-format-code-options
        '(("remove-consecutive-blank-lines?" t)
          ("remove-multiple-non-indenting-spaces?" t)))

And inside this file I have run cider-format-buffer:

(ns temporale.network.test-format)

(def my-map {:a :b :c      :d})

I would expect this file to be reformatted like this

(ns temporale.network.test-format)

(def my-map {:a :b :c :d})

But instead nothing has changed.

I reported this issue on slack a couple of weeks ago and yuhan replied:
This appears to be a bug, the cider--nrepl-format-code-request-map function returns an empty dict

Environment & Version information

CIDER version information

;; CIDER 1.4.0 (Kyiv), nREPL 0.9.0
;; Clojure 1.10.2, Java 1.8.0_332

Emacs version

27.1

Operating system

A pretty recent ubuntu

JDK distribution

openjdk 1.8.0_332

[vemv]

Checking out

cider/cider-client.el

Lines 247 to 272 in 0f8fd4d

	(defun cider--nrepl-format-code-request-map (&optional format-options)
	"Map to merge into requests that require code formatting.
	If non-nil, FORMAT-OPTIONS specifies the options cljfmt will use to format
	the code. See `cider-format-code-options` for details."
	(when format-options
	(let* ((indents-dict (when (assoc "indents" format-options)
	(thread-last
	(cadr (assoc "indents" format-options))
	(map-pairs)
	(seq-mapcat #'identity)
	(apply #'nrepl-dict))))
	(alias-map-dict (when (assoc "alias-map" format-options)
	(thread-last
	(cadr (assoc "alias-map" format-options))
	(map-pairs)
	(seq-mapcat #'identity)
	(apply #'nrepl-dict)))))
	(thread-last
	(map-merge 'list
	(when indents-dict
	`(("indents" ,indents-dict)))
	(when alias-map-dict
	`(("alias-map" ,alias-map-dict))))
	(map-pairs)
	(seq-mapcat #'identity)
	(apply #'nrepl-dict)))))

, it seems like it only will relay indents and alias-map options.

[bbatsov]

Indeed. I don't remember the background, but I'd assume that probably we just weren't very familiar with the options. I know that certainly applies to me. :-)

[yuhan0]

This can be partially fixed by merging the format-options into the dict sent to nrepl, but unfortunately there are upstream issues in nrepl (see nrepl/nrepl#275 (comment)) which make certain options unrepresentable - eg. attempting to set any default-true option to false.

Eg. writing {:indentation? false} in elisp as '(("indentation?" nil)) sends an empty vector, I tried 0 or 'false or "false" and none of them worked.

Edit: the partial fix-

(defun cider--nrepl-format-code-request-map (&optional format-options)
  "Map to merge into requests that require code formatting.
If non-nil, FORMAT-OPTIONS specifies the options cljfmt will use to format
the code.  See `cider-format-code-options` for details."
  (when format-options
    (let* ((indents-dict (when (assoc "indents" format-options)
                           (thread-last
                             (cadr (assoc "indents" format-options))
                             (map-pairs)
                             (seq-mapcat #'identity)
                             (apply #'nrepl-dict))))
           (alias-map-dict (when (assoc "alias-map" format-options)
                             (thread-last
                               (cadr (assoc "alias-map" format-options))
                               (map-pairs)
                               (seq-mapcat #'identity)
                               (apply #'nrepl-dict)))))
      (thread-last
        (map-merge 'list
+                  format-options
                   (when indents-dict
                     `(("indents" ,indents-dict)))
                   (when alias-map-dict
                     `(("alias-map" ,alias-map-dict))))
        (map-pairs)
        (seq-mapcat #'identity)
        (apply #'nrepl-dict)))))

[vemv]

The false thing could be fixed as in refactor-nrepl https://github.com/clojure-emacs/refactor-nrepl/blob/c2f629d82879e12c8976c23b68008bacce72a68b/src/refactor_nrepl/config.clj#L52-L54 i.e. it would need explicit support in cider-nrepl.

Although @bbatsov wasn't a fan of this technique.

[vemv]

Honestly I think that a much cleaner API would be to let users write cljfmt config in .edn format, and honor those files when found. It's what clojure-lsp does, for instance.

[yuhan0]

Would these .edn files then be sent over the wire as bencode strings for cider-nrepl to parse back into Clojure structures? The special casing of "true" / "false" strings in each middleware does seem like an ugly hack.. perhaps there should be some sort of reserved keywords that allow for decoding at a lower level.

I hoped that cljfmt could recognize its own config file, allowing us to sidestep the issue in this case, but it seems that is not implemented weavejester/cljfmt#256

[vemv]

Would these .edn files then be sent over the wire as bencode strings for cider-nrepl to parse back into Clojure structures?

No, cider-nrepl would just read the resource, bypassing any complexity

Here's clojure-lsp's pattern https://github.com/clojure-lsp/clojure-lsp/blob/40b6e70fd4f529d6fdf0b2ec02f11045ab7bbcfd/.cljfmt.edn

The .cljfmt.edn naming is probably 'unofficial' but I do think it would be a good idea to detect that exact file.

That way we would facilitate usages by a variety of users - nobody enjoys duplicated config

[bbatsov]

I'm open to ideas for improving this. Admittedly this functionality never got enough love, mostly because relatively few people use it.

[arichiardi]

Honestly I think that a much cleaner API would be to let users write cljfmt config in .edn format, and honor those files when found. It's what clojure-lsp does, for instance.

Came to say this basically - a recent project I joined uses a .cljfmt.edn in the repo and that my cider did not pick that up.

Another idea would be to shell out but given we do have parseedn I think we are very close to be able to read that file directly.

[zonotope]

I hoped that cljfmt could recognize its own config file, allowing us to sidestep the issue in this case, but it seems that is not implemented weavejester/cljfmt#256

weavejester/cljfmt#256 has been merged, but it still doesn't seem like cider's implementation respects the config file, unless I'm doing something wrong. I have set the cljfmt option to sort ns requires in the .cljfmt.edn in my project root. When I switch two requires so that they're out of order, calling cljfmt from the command line with cljfmt fix <file name> from the directory containing the source file re-sorts the requires, but calling cider-format-buffer from within Emacs does not.

Could this be caused by Cider interacting with cljfmt through nrepl instead of shelling out?