Skip to content

Conversation

@d-torrance
Copy link
Member

As suggested in #77, we decrease the indentation when indenting a line that opens with a ), ], or }. We also revive the unused M2-electric-right-brace function so that we re-indent automatically when typing one of these characters.

Peek 2024-12-19 15-05

@d-torrance d-torrance requested a review from mahrud December 19, 2024 20:14
@mahrud
Copy link
Member

mahrud commented Dec 19, 2024

Amazing! In lieu of a review I will test drive this for a while and report back!

@mahrud
Copy link
Member

mahrud commented Dec 31, 2024

So far happy with this. As long as nobody else is opposed to this change, I'm happy to merge this.

@mahrud
Copy link
Member

mahrud commented Feb 23, 2025

Things are working okay, but I'm getting pretty annoyed when editing older code.

For instance say I start with an old code

f = x -> (
    2 * x
    )
<-- pointer here

Then I press enter to make a new line and add a new function, auto indent gives me:

f = x -> (
    2 * x
    )

    g = x -> (
    )

On the other hand, say I start fresh with:

f = x -> (
    x = 2 * x;) <-- pointer here before the parenthesis

Then I press enter to make a new line and add a new statement, this happens:

f = x -> (
    x = 2 * x;
return x) <-- pointer end up here, before the parenthesis

Now if I press tab I get:

f = x -> (
    x = 2 * x;
return x    ) <-- pointer end up here, before the parenthesis

Which isn't what I wanted ... I need to jump to beginning of the line then tab to indent it correctly.

Not totally sure if these are fixable, but you might know of another option.

It wasn't called anywhere else.  Also add a docstring and simplify the
code.
If the first character in the current line is a right paren, then we
decrease the indentation.

Rather than basing our indentation on just running parse-partial-sexp
on the previous line, we now determine it from the paren depth using
syntax-ppss.

Also delete the now-unused function M2-next-line-indent-amount.
Its keybinding to "}" has been commented out for ages, but it will be
very useful for re-indenting lines that begin with a right paren.

In addition to "}", we also bind it to ")" and "]".  We also remove it
from the menu since it's useless without typing a specific key.
Previously, we added additional tabs/spaces at the point, but now we
re-indent the line, which is the usual behavior in many other major
modes.

We add a new helper function, M2-indent-line, which we can also use to
simplify M2-electric-right-brace.
@d-torrance
Copy link
Member Author

I finally played around with this some more. I think I've addressed both issues! In particular:

  • The indentation is now calculated based on the paren depth rather than just the paren change in the previous line and the current indentation. So it shouldn't get messed up even if nearby code isn't indented properly.
  • Tab'ing when to the right of non-whitespace text now re-indents the line.

@mahrud
Copy link
Member

mahrud commented Apr 13, 2025

Great, I'll give it a try!

@mahrud
Copy link
Member

mahrud commented Apr 13, 2025

By the way, it would be good to bring this up in Tulane.

@d-torrance
Copy link
Member Author

It would be. Will you be there? I won't

@mikestillman
Copy link
Member

I'll be there. What all should I say about it? When will it take effect for people's code? Probably for emacs, and when they install the next version?

@mikestillman
Copy link
Member

I need to try this, but it sounds good!

@d-torrance
Copy link
Member Author

@mikestillman - If this is merged, then it would appear with the next release in May.

I put together a short video demonstrating the proposed changes: https://youtu.be/o28KrBMWa2w

@d-torrance
Copy link
Member Author

One big disadvantage of the latest change (determining indentation based on the global paren depth rather than just locally looking at the previous line's indentation and paren change) is that it messes SimpleDoc docstrings up. Since they're not surrounded by any parentheses, it thinks that all the lines should start at the very beginning, with no indentation.

An easy workaround is to just use C-j instead of RET, but this may be a deal breaker.

@d-torrance
Copy link
Member Author

Actually, the SimpleDoc issue might have been a problem even in the first iteration of this PR...

I'm thinking of adding an additional "are we inside a ///-delimited string?" function (maybe using font-lock), and if so, adjust the indentation rules accordingly. Probably nothing too fancy -- maybe Tab gives us 2 spaces and we don't re-indent automatically.

@mahrud
Copy link
Member

mahrud commented Apr 15, 2025

I'm thinking of adding an additional "are we inside a ///-delimited string?" function (maybe using font-lock), and if so, adjust the indentation rules accordingly. Probably nothing too fancy -- maybe Tab gives us 2 spaces and we don't re-indent automatically.

Perhaps falling back on deciding indentation based on previous line in this case is best, so code inside examples can be properly indented as well.

@mahrud
Copy link
Member

mahrud commented Apr 18, 2025

Do you happen to have a fix for the SimpleDoc issue? No worries if you haven't had a chance to look at it yet.

@d-torrance
Copy link
Member Author

I've played with it a bit, but nothing working yet.

@mikestillman
Copy link
Member

I am generally a person who is used to the older way of doing this (ie. indent to below last lines, not one level out). I need to play with it. In some sense this is ok, and seems more bullet-proof. But I am a skeptic who needs a bit of convincing perhaps...

Here is something I don't know how easily to do with the new indentation rules. If I type n//2, then go to the next line, it reindents to under the if. Not sure what I think about that. I guess if I want to always use parens for if-then-else, that might be ok. But otherwise, I have to move off the line first, and make sure I don't re-indent the region. What do you think?

f = (n) -> (
    if even n then
        n // 2

@mikestillman
Copy link
Member

I found your video useful too...!

@d-torrance
Copy link
Member Author

Yeah, I see what you're saying. I usually use parentheses every time I want the indentation to increase, but I see the appeal of omitting them.

@mikestillman
Copy link
Member

this is a problem also in the current M2 file, in that it won't indent it. However, it doesn't move it on you either. I'm not sure what people will think about that. It is nice to do it automatically, but I'm worried about what users will think. They might be fine, they might not. By the way, at Tulane, I would say most people appeared to be using vscode. So I'm not sure if they would be good ones to ask about an emacs thing. We should have some other people who use it alot try it. Maybe Greg, Anton, David. I'll ask Dan too.

@mahrud
Copy link
Member

mahrud commented Apr 21, 2025

By the way, at Tulane, I would say most people appeared to be using vscode. So I'm not sure if they would be good ones to ask about an emacs thing. We should have some other people who use it alot try it. Maybe Greg, Anton, David. I'll ask Dan too.

The point of this PR is to bring the indentation behavior in Emacs in-line with what vscode already does, and avoid having two editors that indent code differently. The alternative would be to figure out how to change indentation rules in the vscode extension.

@mahrud
Copy link
Member

mahrud commented Apr 21, 2025

@d-torrance FYI I was too frustrated with the current version of this PR unindenting my tests left and right, so I switched back to the older version of this PR, which in comparison was much less frustrating. If possible, I think the following is a decent compromise, if it's doable:

  • on a given line, if the previous line is empty, use global indentation.
  • otherwise inherit indentation baseline from the previous line.

@d-torrance
Copy link
Member Author

I'll play around with this some more soon.

FYI -- by default, VS Code automatically closes parens, e.g., if you type "(", it automatically inserts a closing ")". You can replicate this in Emacs using M-x electric-pair-mode. Maybe this is worth putting in doc somewhere.

@mikestillman
Copy link
Member

@d-torrance FYI I was too frustrated with the current version of this PR unindenting my tests left and right, so I switched back to the older version of this PR, which in comparison was much less frustrating. If possible, I think the following is a decent compromise, if it's doable:

  • on a given line, if the previous line is empty, use global indentation.
  • otherwise inherit indentation baseline from the previous line.

I also found the changing indentation in doc nodes to be hard to work with, so I had to revert back to the old behavior too.

I personally don't like filling in closing brace too when e.g. ( is pressed, but for those who are used to it, it might be nice to have that as an option. For me, I would not want that to be the default.

@mahrud mahrud removed their request for review September 8, 2025 23:27
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