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

Inconsistent indentation for custom threading macros #3684

Closed
DerGuteMoritz opened this issue May 27, 2024 · 7 comments · Fixed by clojure-emacs/cider-nrepl#884
Closed

Inconsistent indentation for custom threading macros #3684

DerGuteMoritz opened this issue May 27, 2024 · 7 comments · Fixed by clojure-emacs/cider-nrepl#884

Comments

@DerGuteMoritz
Copy link
Contributor

Expected behavior

Custom threading macros (like the ones found in promesa) should be indented like the ones from clojure.core when operands are given in separate lines, i.e. they should line up vertically like so:

(p/->> 1
       inc
       inc)

Actual behavior

Instead, CIDER indents such custom macros like a macro with a "head" form (e.g. like doto):

(p/->> 1
  inc
  inc)

This might be a regression of the fix introduced for #3490.

Steps to reproduce the problem

  1. Checkout https://github.com/bevuta/cider-inconsistent-indentation-for-custom-thrush-macros/ - this repo is a self-contained reproducer which doesn't rely on any external dependencies (such as promesa)
  2. Start a CIDER REPL via M-x cider-jack-in from the project root and select the clojure-cli command
  3. Open src/repro/core.clj and re-indent the main function in there
  4. As you can see, the p/ok->> form will be indented like clojure.core/->> but the p/not-ok->> form won't. If you look at their definitions in src/repro/p.cljc you can see that the only difference is that not-ok->> has a standalone first argument while ok->> doesn't. This also happens when renaming not-ok->> to just ->> (as is the case in promesa).

This also happens with ClojureScript which can be tested by M-x cider-jack-in-cljs and selecting the :main build.

Environment & Version information

CIDER version information

;; CIDER 1.13.1 (Santiago), nREPL 1.0.0
;; Clojure 1.11.1, Java 21

Lein / Clojure CLI version

Clojure CLI version 1.11.3.1463

Emacs version

GNU Emacs 29.3 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.18.0, Xaw3d scroll bars)

Operating system

NixOS 24.05

JDK distribution

OpenJDK

@vemv
Copy link
Member

vemv commented May 27, 2024

Thanks, I'll check.

@vemv
Copy link
Member

vemv commented May 27, 2024

The bug is that one should be able to specify {:style/indent nil} and have cider-nrepl honor that explicit specification.

Most likely we conflate "nothing was specified at all" with "nil was specified".

I'll try to fix this in the first half of the week, please do ping us if that doesn't happen.

(PR also welcome for anyone wanting to grab it)

@DerGuteMoritz
Copy link
Contributor Author

Friendly ping 😄 No hurry, though. I'll also try to look into it using your hint when I find some spare cycles again. But that might be a while.

@vemv
Copy link
Member

vemv commented Jun 13, 2024

You can check out clojure-emacs/cider-nrepl@7fdca13 and run:

export LEIN_JVM_OPTS="-Dmranderson.internal.no-parallelism=true"
PROJECT_VERSION=0.49.0 make install

Change, if needed, 0.49.0 to whatever version of cider-nrepl you normally use.

Then, adding {:style/indent nil} should work.

LMK how it goes - thanks!

@DerGuteMoritz
Copy link
Contributor Author

Thanks!

Then, adding {:style/indent nil} should work.

Where would I add that? To the macro's metadata? If so, then this won't do the trick for third-party macros, e.g. promesa.core/->>, right?

@DerGuteMoritz
Copy link
Contributor Author

(Unless they are willing to include it upstream, that is)

@vemv
Copy link
Member

vemv commented Jun 13, 2024

To the macro's metadata?

Yes. You can alter-meta as a local experiment.

After we conclude it works, I can PR it myself in case a generous explanation helps th change making it in.

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

Successfully merging a pull request may close this issue.

2 participants