justinbarclay / parinfer-rust-mode

Simplifying how you write Lisp
https://shaunlebron.github.io/parinfer/
GNU General Public License v3.0
227 stars 16 forks source link

evil-open-above messes up indentation of code below in smart mode #56

Closed johanthoren closed 5 months ago

johanthoren commented 2 years ago

I've tested this on both Spacemacs and Doom Emacs running Arch Linux.

Peek 2021-09-28 14-27

justinbarclay commented 2 years ago

Hey, thanks for this, I will try to look into this on the weekend and see if I can come up with anything.

johanthoren commented 2 years ago

Let me know if you need anything.

gilch commented 2 years ago

I ran into this one too, on Spacemacs. Any progress or workarounds? If this were Vim I might try something like

:nmap O 0i<ENTER><UP>

But I don't know how to do this in Evil. At least, it still didn't work right when I tried it.

gilch commented 2 years ago

Turning off evil-auto-indent is a workaround. You can still tab after opening the line.

justinbarclay commented 2 years ago

Thanks for doing some more investigating on this. I tried to play around in this space a while back by running emacs -Q with only parinfer-rust and evil-open-above, but I could not recreate the bug.

The issue you are describing now is definitely a known issue for parinfer: it does not play nicely with other systems that want to manage whitespace.

However, I did build an escape hatch for commands that don't play well with parinfer which might work for this situation:

parinfer-rust-treat-command-as

You could try to add evil-auto-indent to this list and see if it starts working as expected.

gilch commented 2 years ago

I'm on Spacemacs develop, and this is noticeable when editing the elisp dotfile SPC f e d. I'm trying to narrow down a minimal example text that demonstrates the issue.

I've noticed that cc has the same issue.

I'm getting errors trying to use parinfer-rust-treat-command-as, specifically "Symbol's value as variable is void: Parinfer-rust-treat-command-as". Maybe some kind of Spacemacs lazy-loading issue.

gilch commented 2 years ago

OK, I found a pretty small example. Make a new empty foo.el file. Then given this text and point,

(defun foo ()
█ ()
  x)

The O command results in

(defun foo ()
  █
  (
    x))

And given

(defun foo ()
█ ;;
  ()
  x)

The cc command results in the same.

gilch commented 2 years ago

OK adding (add-to-list 'parinfer-rust-treat-command-as '(evil-open-above . "paren")) to my dotspacemacs/user-config fixes the O command, but I have to call (parinfer-rust-mode) immediately above it or the list doesn't exist.

I'm not sure what to add to fix cc though. The command bound to c (evil-change) didn't work. Is that the wrong command because there are two keystrokes? (setq-default evil-auto-indent nil) worked for cc as well, but I'd rather not have to turn that off.

I'm wondering if there isn't a more generic fix for this. cc and O probably aren't the only commands affected. It seems like a race condition. I thought Emacs was pretty much single-threaded, but I'm not sure how that works with Rust. Is Evil adding the indents before or after Parinfer makes changes? Is there some way to lock the buffer to prevent conflicts, or abort committing Parinfer's changes back to the buffer if the buffer contents don't still match the input originally given to Parinfer?

gilch commented 2 years ago

evil-auto-indent is only documented to affect opening lines with o and O. I don't understand why that helps with cc.

gilch commented 2 years ago

evil-change-whole-line is also affected. I have it bound to S, which (I think) is the default for Spacemacs, but not Evil. Adding that to the list seems to fix cc as well.

justinbarclay commented 2 years ago

OK adding (add-to-list 'parinfer-rust-treat-command-as '(evil-open-above . "paren")) to my dotspacemacs/user-config fixes the O command, but I have to call (parinfer-rust-mode) immediately above it or the list doesn't exist.

Unfortunately, I have no clue how spacemacs works or how it loads things, so there is not much I can do to fix that. The alternative would probably be to add that command to a mode hook for parinfer.

Warning I haven't tested this command, but I think this is right:

(add-hook 'parinfer-rust-mode-hook
          (lambda () (if parinfer-rust-enabled
                         (add-to-list 'parinfer-rust-treat-command-as '(evil-open-above . "parent")))))

I'm wondering if there isn't a more generic fix for this.

Maybe, but I didn't find a way when I was initially building this and if you asked this question 2 years ago I might have been able to explain my choices better 😅.

However, I'm not the only one who, in the end, used a list of commands that needed to have different rules apply to them. If someone can find a better solution to the problem, I am more than happy to accept a pull request.

It seems like a race condition. I thought Emacs was pretty much single-threaded, but I'm not sure how that works with Rust. Is Evil adding the indents before or after Parinfer makes changes?

It is almost certainly a race condition. It could be around when change tracking occurs and assumption of the current buffer state, or based on when the post-command-hooks are processed?

gilch commented 1 year ago

I have been using

(parinfer-rust-mode)
(add-to-list 'parinfer-rust-treat-command-as '(evil-open-above . "paren"))
(add-to-list 'parinfer-rust-treat-command-as '(evil-change-whole-line . "paren"))

for a while now and haven't run into any obvious problems with that.

Just now I tried

(add-hook 'parinfer-rust-mode-hook
          (lambda ()
            (when parinfer-rust-enabled
              (add-to-list 'parinfer-rust-treat-command-as '(evil-open-above . "paren"))
              (add-to-list 'parinfer-rust-treat-command-as '(evil-change-whole-line . "paren")))))

in a new config, and it seems to work just as well.

Perhaps these can be added to the default parinfer-rust-treat-command-as list.

justinbarclay commented 1 year ago

Sounds good, do you want to open a PR?