Clean Up, Modernize, and Debug Indentation

[Gastove]

Description

Indentation comments on the issue tracker here are somewhat common, and difficult to resolve. fsharp-mode has not one but two indentation modules -- fshap-mode-indentation and fsharp-mode-indentation-smie. fsharp-mode-indentation, in particular, contains a great deal of historic cruft, from blocks of unexplained comments to totally unrelated functions.

lsp brings with it a lot of very appealing functionality, including F# formatting. And: fsharp-mode still needs to be able to handle indentation well along several lines. Partly, this is because it makes writing F# using Emacs vastly more pleasant to have good, correct indentation support. More concretely, however, is the fact that in F#, whitespace is syntactically significant and often computationally un-guessable.

Currently, in fsharp-mode, there are cases in which it is impossible to get correct indentation without forcing it. Consider an example from an Expecto test:

let tests =
    testList "Serialization Round-Tripping"
        [ testCase "beep" <| fun _ ->
          |]

fsharp-mode will not indent point (indicated by |) past the exact column of the t in testCase.

Proposal

I'd like to work on this, and I propose to do the following:

1. Verify that smie configs are correct, and being applied correctly.
2. Remove unneeded comments, unused code.
3. Move code not related to indentation out of fsharp-mode-indentation -- some to a new module, some to fsharp-mode.
4. Start a slow refactor to remove unneeded code.
5. Modify indentation code to behave... better, at least.

This is absolutely more than one PR.

In terms of changes to make: "unneeded code" is a much harder question to answer than I'd like. smie configures some things duplicated in fsharp-mode-indentation. I expect, in general, to remove some amount of code from fsharp-mode-indentation, beyond removing i.e. functions not related to indentation. When this is done, perhaps the smie configs can be merged in to fsharp-mode-indentation sensibly.

Indentation Issues to be Resolved by This Work

• [ ] #41
• [ ] #211
• [ ] #68

[Gastove]

Oookay. The more I work on pulling apart fsharp-mode-indent, the messier it gets. Everything in there... references everything else in there. Untangling it is certainly possible, but it will be a very, very messy PR.

Rather than open such a mess, I propose instead to rename fsharp-mode-indent to fsharp-mode-structure, and to pull fsharp-mode-indent-smie into it.

[Gastove]

@juergenhoetzel: I'd love eyes or commentary on this, if anyone has the time? Yourself or another maintainer?

[geoffder]

Having started to write F# in emacs recently, I have to say the auto-indentation experience is incredibly frustrating (as mentioned here and in the linked bug issue)!

:+1: to this issue.

[Gastove]

Okay. I'm Working on this again; sorry about the long delay. It turns out, this is very difficult to reason about! Or, at the very least, I find it hard. It's not like a lot of other problems I've solved.

My current worldview goes like this:

fsharp-mode should offer indentation support that allows semantically correct code. This means the programmer should be able to line up one block of code with any preceding block. However, for general formatting, we should lean on a formatter, like Fantomas, either via an LSP server or by running it as a dotnet tool.

I'm curious if the block traversal, "go to end of block"-style functions are used by many folk. They are finicky and hard to get right; things like go-to-next-let-same-level might be a bit easier to reason about.

Anywho. I've got code that mostly implements the above, though my tests are mostly ERT, so I need to learn and switch to buttercup. I'll try and get a PR up for discussion soon.

[theothornhill]

@Gastove Did you get any further on this? If you want, I can help out in implementing this. I think we should have an indentation calculation that is 'good enough' without formatters.

I was frustrated with the current elm-mode which was basically haskell-mode, so I made my own: https://git.sr.ht/~theothornhill/elmo/tree/master/elmo.el

What's good about this is that it guesses only the indentation levels that are obvious and easy. Otherwise we can rely on tab and backtab. This way it behaves like vscode, rider etc and being way, way simpler.

I think we could remove the smie indentation altogether, and just implement some simple 'go to nearest let' functionality.

What do you think?

[razzmatazz]

I think we could remove the smie indentation altogether, and just implement some simple 'go to nearest let' functionality.

I agree that current indentation with or w/o smie is just bad.. Maybe I am missing some crucial configuration bits, but it is just not usable for me and I have to hack most of it out in init.el just to keep things sane by relying on custom code to:

• on newline-- keep indentation on the same level as previous line, dont guess anything
• tab+backtab to indent+unindent line level

[theothornhill]

Saulius Menkevičius @.***> writes:

I think we could remove the smie indentation altogether, and just implement some simple 'go to nearest let' functionality.

I agree that current indentation or and w/o smie is just bad.. Maybe I am missing some crucial configuration bits, but it is just not usable for me and I have to hack most of it out in init.el just to keep thing sane by relying on custom code to: - on newline-- keep indentation on the same level as previous line, dont guess anything - tab+backtab to indent+unindent the lines

Yeah. Right now I am using a home-grown major mode instead of this one. It does what you suggest here, only simple fontification and super simple indentation. It isn't ready to be shown to anyone yet, I think, but I use it daily at $DAYJOB. I'm not sure how to best contribute, since this issue seems somewhat stale, and my attempt is a rewrite with most likely its own bugs and issues.

Would simplifying the indentation engine to almost nothing be desirable here? Surely we can do better :)

-- Theo

[juergenhoetzel]

Would simplifying the indentation engine to almost nothing be desirable here? Surely we can do better :)

I agree that the existing code is "unmaintainable". Any minimal clean rewrite PR is welcome.

I used this hook to auto-format when saving a buffer:

(use-package eglot :ensure t
  :hook
  (fsharp-mode . (lambda () (add-hook 'before-save-hook 'eglot-format-buffer nil 'local))))