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

Various indentation fixes: typos, differentiating new lines, support for do and extension #7

Merged
merged 7 commits into from
Jul 6, 2023

Conversation

maemre
Copy link

@maemre maemre commented Jun 5, 2023

This PR contains the following fixes to support Scala 3 indentation (numbers 2 and 3 made it way more usable in my experience):

  1. Some fixes with propagation of nil values and typos.
  2. Disabling emitting the left-boundary token when encountering a left paren: (. This turned out to be too aggressive when the indentation engine needs to look past an open paren to hit a token like class, case and so on.
  3. Distinguishing blank lines and the previous non-blank line. This allows proper indentation when hitting return after a class declaration, etc.
  4. Adding some support for indenting do and extension keywords properly:
  • do is treated just like yield (or then) because there are no more do..while loops in Scala.
  • extension needs to be handled specially because extension blocks don't start with a : or = (unlike classes or declarations). Current implementation uses a rough heuristic (observing extension and an occurrence of a newline).

There are still some fixes that need to be made (I can move this bit to hvesalai#160):

  • I haven't looked at using the cycling behavior. So far, I have focused on making a good initial guess.
  • There is no support for the end keyword. The simplest approach would be to make it reduce indentation by one level, but there are variants like end if, end foo which should scan for the corresponding unclosed block.
  • There is an edge case for when re-indenting the statement after the end of a block. Current heuristics assume that this statement would be a continuation of the previous block (I included an example before). For comparison, haskell-mode does not change the indentation in this case.

Here is an example of the last thing I mentioned:

for x <- xs do
  print(x)
  for y <- x.bars do
    val x = print(y)
  print(s"end of $x")  // if we open a new line after this statement, the
                       // indentation heuristics indent both lines to align
                       // with print(y)

After this PR, I can focus on adding support for the end keyword, but solving the last problem would be a bit difficult (it is effectively trying to guess the user's intent). In my opinion, using cycling+a sane default would be the best option there.

maemre and others added 7 commits June 4, 2023 19:41
In Scala 3, `do-while` is dropped, and `do` is used only for separating the body of a `while` or `for` from the guard/comprehension. So, `do` would need to be indented like `then` or `else`. See: https://docs.scala-lang.org/scala3/reference/dropped-features/do-while.html#
This allows differentiating between indenting a blank line and re-indenting the
line before it.  This situation occurs fairly often: it happens every time the
user hits re turn to create a new line, which calls indentation function on the
previous line and the newly created line separately.  Before this fix, these two
calls would incorrectly calculate the same indentation level, e.g. when the
previous line just created a block.
@pauloglesby
Copy link

Any update on this, folks? It would be awesome to have better scala3 support in emacs 🙏

@maemre
Copy link
Author

maemre commented Jun 30, 2023

@Kazark this is ready for review, in case it went unnoticed so far (I couldn't explicitly request a review through the GitHub UI).

After the 4th of July, I'll have some time to work on supporting end and testing out interactions with cycling.

@Kazark
Copy link
Owner

Kazark commented Jul 6, 2023

Yep, somehow I missed this when it first submitted. Apologies! Will take a look now.

Copy link
Owner

@Kazark Kazark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not writing a lot of Scala right at the moment, and too busy to make time to specifically test this. However, everything I see here appears to exhibit an understanding of the existing code, and to be a movement in a good direction. Sorry for the slow review, and keep up the good work!

@@ -653,7 +663,7 @@ Returns point or (point-min) if not inside a block."
(setq last-indentation (current-indentation))
(while (looking-at-p "\\.") (backward-char))
;; ")." is a funny case where we actually do want to be on the dot
(if (looking-at-p ")") (forwar-char))
(if (looking-at-p ")") (forward-char))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes, how did I miss that

;; Get the line number while taking blanks into account. This allows
;; differentiating between indenting at a blank line and re-indenting
;; at the line right before it.
(line-number-at-pos
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought. This extra piece of information seems useful.

@Kazark Kazark merged commit 238d4ea into Kazark:scala3 Jul 6, 2023
@pauloglesby
Copy link

This is great - thanks @maemre & @Kazark 🙌 Looking forward to trying this out on some Scala 3 next week 😄

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 this pull request may close these issues.

3 participants