Simplifying elm-mode

[theothornhill]

Hello again!

As discussed in #175, let's start discussing how to merge/simplify this.

With these commits, things shouldn't really be that controversial.

What I've done:

• Remove stuff that's obviously not needed since elm-language-server
• Add my own indentation logic - not enabled yet.
• Don't remove elm-interactive just yet, though most of it can go

To enable my indentation logic, I've added this snippet to my own config:

(use-package elm-mode
  :ensure nil
  :bind (:map elm-mode-map
              ("C-c TAB" . #'elm-indent-line)
              ("<tab>" . #'elm-indent-forward)
              ("<backtab>" . #'elm-indent-backward)))

(remove-hook 'elm-mode-hook 'elm-indent-mode)

This indentation system doesn't try to be smart at all. There are some defaults for the easiest cases that are simple to get right, other than that, just use tab and backtab. This may be a little controversial given how emacs tries to be super smart, but the hassle is often not worth it IMO.

And remember, this is merely a starting point for discussion. Everything can be discussed :) Have a nice day!

[theothornhill]

Firstly - thanks for the review so far!

Thanks, some good stuff here, but I feel like it's starting in the wrong place and likely to disrupt many workflows. I don't think we should start by assuming LSP support when there is working support for direct integration of common tools like elm-format that might suit many people's workflows better for now. Also imenu support is generally straightforward and is reasonably expected of a major mode.

Right - thats fine by me. Does the elm-package integration work on your end? Has it ever worked? Right now I'm getting some (wrong-type-argument listp (0ui/elm-task-parallel . ["1.0.0" "1.0.1" "1.0.2" "2.0.0"])) when trying to invoke M-x elm-package-catalog. I'm wondering because if it isn't working, that whole thing could go, since probably not many people are using it 😄, otherwise, I could try to debug it a little. It seems like let-alist wants an alist, but is getting a normal cons pair, which isnt a list.

Simplifying/improving indentation code etc. is a less controversial first step, as would be dropping support for 0.18, including all the elm-oracle machinery, which won't work with 0.19+ anyway.

Ok sure. Will create a new commit when I get some free time on my hands. elm-indent is a beast, so not really sure how to approach that.

[purcell]

Right - thats fine by me. Does the elm-package integration work on your end? Has it ever worked? Right now I'm getting some (wrong-type-argument listp (0ui/elm-task-parallel . ["1.0.0" "1.0.1" "1.0.2" "2.0.0"])) when trying to invoke M-x elm-package-catalog. I'm wondering because if it isn't working, that whole thing could go, since probably not many people are using it 😄, otherwise, I could try to debug it a little. It seems like let-alist wants an alist, but is getting a normal cons pair, which isnt a list.

Hmm, I'd probably get the same result as you locally. Ideally we'd keep that functionality (but working, obviously) if possible, because it seems broadly useful.

Simplifying/improving indentation code etc. is a less controversial first step, as would be dropping support for 0.18, including all the elm-oracle machinery, which won't work with 0.19+ anyway.

Ok sure. Will create a new commit when I get some free time on my hands. elm-indent is a beast, so not really sure how to approach that.

Maybe make an elm-indent-simple for now, since I think you were talking about making it pluggable initially?

[theothornhill]

Sure, I'll make elm-indent-simple, and try to look into the elm-package stuff after that.

Maybe we'll just add the elm-indent-simple as a part of this pr, in addition to removing <0.19 stuff, then start working on the other stuff incrementally?

[purcell]

Yeah, sure, or separate small PRs are equally fine, and often easier to review and just merge in successively.

[theothornhill]

Is the current PR more akin to what you want?

(use-package elm-mode
  :ensure nil
  :config
  (setq elm-mode-enable-indent-simple t))

This snippet changes the indentation method, and it defaults to the current method.

It should be quite non-intrusive as for now, I think :)

[theothornhill]

I think I've addressed all your comments - thanks :)

Also, now you have to do this in your config:

(use-package elm-mode
  :init
  (setq elm-mode-indent-mode 'elm-indent-simple-mode))

What do you think?

[theothornhill]

@purcell Hey there, did you get a chance to take a look? :)

[theothornhill]

Great, thanks!

[purcell]

I updated the README after this but there might be other occurrences.