search

clojure-emacs / clojure-ts-mode

The next generation Clojure major mode for Emacs, powered by TreeSitter
GNU General Public License v3.0
140 stars 13 forks source link

Semantic Indentation #9

Closed dannyfreeman closed 1 year ago

dannyfreeman commented 1 year ago

Clojure-ts-mode currently only uses a simple fixed indentation style made up of two basic rules taken from Tonsky's blog post on Clojure indentation [0].

This was a great way to get clojure-ts-mode off the ground. However, most Clojure developers are adjusted to formatting rules similar to those outlined in the Clojure Style Guide [1].

This ticket is for a feature to allow users to choose between the simple fixed indentation rules that exist now, and semantic indentation rules that match (as closely as possible) those laid out in the Clojure Style Guide.

[0] Better Clojure Formatting [1] Clojure Style Guide

dawranliou commented 1 year ago

Hey @dannyfreeman thank you for the great work! I've been using clojure-ts-mode as my daily driver since I saw the announcement blog post. It solved a pain point for me when working with humongous map literals. 🙏 Would you share how much collaboration you seek for this project? Would you like bug reports, or do you accept PRs at the current stage of the project?

Just for the people looking for a way to hack the formatting rules to comply with the Clojure Style Guide [1] before this issue #9 is resolved, here's the relevant code snippet from my config:

(with-eval-after-load 'clojure-ts-mode
  (require 'clojure-mode)
  (add-hook 'clojure-ts-mode-hook #'clojure-mode-variables)
  ;; ...
  )

This sets up many clojure-mode local variables just as the clojure-mode does, most noticeably the code and comment formats.

dannyfreeman commented 1 year ago

It solved a pain point for me when working with humongous map literals.

Happy to hear this is doing some good out in the real world!

Would you share how much collaboration you seek for this project? Would you like bug reports, or do you accept PRs at the current stage of the project?

I haven't given it much thought, so far I have only received small bug reports. Happy to look at PRs though. Currently the only things I'm really thinking about are this semantic indentation issue, as well as bug fixes and improvements to the current code.

I will say that I have a pretty busy schedule between my work and my family. Usually I only have a couple weekend hours to get work done on this. If you end up opening a PR it may take some time for me to get around to reviewing it, but I will get around to it eventually.

jasonjckn commented 1 year ago

What do we have for prior art, for complex indentation rules, I know https://github.com/mickeynp/combobulate has complex indentation, as I recall, it's doing a lot of calculations crawling the treesit node/graph and working directly with the AST. Is this kind of what we had in mind? Or are there simpler ways to accomplish complex indentation rules. Combobulate indentation works really well in practice for languages it supports like Python, et al, but i'm still trying to wrap my head around its source code.

dannyfreeman commented 1 year ago

I haven't had much chance to look at it, but the plan was to see how much I could get away with implementing using the provided simple indent functionality. I would hope that is enough to say stuff like "if the parent list first child is a ->, indent with this special rule". I have no idea if the simple indent rules mechanisms will be flexible enough to do this. Other languages much more complex than clojure have gotten away with just using them so far

dannyfreeman commented 1 year ago

Work in progress branch for implementing semantic indentation: https://github.com/clojure-emacs/clojure-ts-mode/compare/semantic-indentation

(Warning: I love to rebase branches as I work, commit history is subject to change)

dannyfreeman commented 1 year ago

My initial plan was to allow 2 formats.

  1. Fixed indentation (currently on main branch) 'fixed
  2. Semantic indentation 'semantic

But when looking at clojure-mode and cljfmt I notice some differences. I'm thinking of creating three preconfigured nodes now

  1. Fixed indentation 'fixed
  2. Cljfmt default settings indentation 'cljfmt
  3. Clojure-mode style indentation 'clojure-mode

I don't think it will be too bad to implement both, I'll see what that looks. I think cljfmt would be the default.

EDIT: I have a setting (setq clojure-indent-style 'align-arguments) in my emacs configuration that is not the default. I think default clojure-indent-style is more in line with cljfmt. Maybe I turned that on to fight less some former coworkers using intellij? Either way, I will at least name the second formatting style cljfmt instead of semantic to be clear about what it should do

jasonjckn commented 1 year ago

Looking good so far... will you be able to support 'clojure-align-forms-automatically t' as well?

That's been the biggest usability issue for me with clojure-ts-mode, because I can always use on-save formatter like https://github.com/radian-software/apheleia with cljfmt + clojure-ts-mode to regain the cljfmt functionality, but clojure-mode.el is the only formatter i'm aware of with 'clojure-align-forms-automatically' indenting.

jasonjckn commented 1 year ago

I was playing around with lisp-indent-function, and it seems to matter greatly what it is set to... I don't understand why that matters, but beware of it, here's the experiment

https://github.com/clojure-emacs/clojure-ts-mode/assets/550839/8613f35d-bdb6-4601-8cb5-e95117835133 higher res: https://drive.google.com/file/d/191_I0IwfxYVrv1BRCoiGgFSGtl0ln7xz/view?usp=sharing

(defun my/set-lisp-indent-lisp ()
  (interactive)

  (setq-local
  lisp-indent-function #'lisp-indent-function ; <-----------------

   indent-line-function #'treesit-indent
   indent-region-function #'treesit-indent-region
  ))

(defun my/set-lisp-indent-clojure ()
  (interactive)

  (setq-local
  lisp-indent-function #'clojure-indent-function ; <-----------------

   indent-line-function #'treesit-indent
   indent-region-function #'treesit-indent-region
  ))

(defun my/set-lisp-indent-treesit ()
  (interactive)

  (setq-local
  lisp-indent-function #'treesit-indent ; <-----------------

   indent-line-function #'treesit-indent
   indent-region-function #'treesit-indent-region
  ))
dannyfreeman commented 1 year ago

Looking good so far... will you be able to support 'clojure-align-forms-automatically t' as well?

That's been the biggest usability issue for me with clojure-ts-mode, because I can always use on-save formatter like https://github.com/radian-software/apheleia with cljfmt + clojure-ts-mode to regain the cljfmt functionality, but clojure-mode.el is the only formatter i'm aware of with 'clojure-align-forms-automatically' indenting.

I don't think that will be part of the initial pass at semantic indentation. But it is probably do-able? I think we make a new issue for this.

I was playing around with lisp-indent-function, and it seems to matter greatly what it is set to... I don't understand why that matters, but beware of it, here's the experiment

Clojure-ts-mode doesn't use lisp-indent-function (i'm 90% sure). When it's value is the function from clojure-mode we'll want to match that. Either way, it is a lot to watch out for.

dannyfreeman commented 1 year ago

The primary work for this is DONE now I believe. It is enabled by default on the main branch. A release has not yet been cut, but is available for testing (and should be on unstable melpa soon enough). Please open a new issue to report bugs with this.

I am currently aware that indentation within docstrings is a little too aggressive. I'm not sure how to handle it but I will open a new issue to track it.