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

Clean Up, Modernize, and Debug Indentation #218

Open
3 tasks
Gastove opened this issue Oct 20, 2019 · 8 comments
Open
3 tasks

Clean Up, Modernize, and Debug Indentation #218

Gastove opened this issue Oct 20, 2019 · 8 comments

Comments

@Gastove
Copy link
Contributor

Gastove commented Oct 20, 2019

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

@Gastove
Copy link
Contributor Author

Gastove commented Oct 20, 2019

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
Copy link
Contributor Author

Gastove commented Oct 29, 2019

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

@geoffder
Copy link

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)!

👍 to this issue.

@Gastove
Copy link
Contributor Author

Gastove commented Jun 18, 2020

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
Copy link

@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
Copy link

razzmatazz commented Jul 13, 2021

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
Copy link

theothornhill commented Jul 20, 2021 via email

@juergenhoetzel
Copy link
Collaborator

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))))

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

No branches or pull requests

5 participants