eraserhd / parinfer-rust

A Rust port of parinfer.
ISC License
540 stars 41 forks source link

Parinfer Does Not Ignore Parens in String Literals [Spacemacs/Emacs] #139

Open hrestey opened 1 year ago

hrestey commented 1 year ago

I first noticed this with a regex expression that was getting messed up in Clojure, but it appears to be the case for normal string literals as well.

Type "()", delete closing parens, and parinfer edits it to "("). It's not often that I have to use parens inside of strings, but for a regex it is obviously something that happens frequently. Regexes of course do not follow the same syntax as a lisp, so this is obviously quite undesirable.

eraserhd commented 1 year ago

Hrmm... it is definitely supposed to, and this doesn't happen for me. Is it possible there's another plugin doing something weird?

eraserhd commented 1 year ago

To be clear, I tested in a different editor because I don't have a Spacemacs setup at the moment.

garrett-hopper commented 1 year ago

I am able to reproduce this on the latest master branch using the parinfer-rust emacs plugin.

It does not occur when switching to v0.4.3

garrett-hopper commented 1 year ago

Appears to be a regression between 0789c485 and 60204aba

Last buildable commit that doesn't have issue: 0789c4852d09d51ad66b81c44ce2575f421cd031 (There's a few unbuildable commits in between) First buildable commit that introduces issue: 60204aba3ce375b9ec1f9b90756f62746b41419c

gagbo commented 1 year ago

I just started noticing this as well in Emacs, it messes up a lot of files, thanks for finding the commit to revert to.

gagbo commented 1 year ago

FTR, the only meaningful difference I saw at first glance is the pattern matching order changing in the src/parinfer.rs:944 https://github.com/eraserhd/parinfer-rust/compare/0789c4852d09d51ad66b81c44ce2575f421cd031...60204aba3ce375b9ec1f9b90756f62746b41419c#diff-12c8375c7f73fa70fdbbaa8f23ed708403b0199da0779cf58c4a83d348370449R946

What I don't get, is how can we be hit by a regression now while the change is 2 years old.

garrett-hopper commented 1 year ago

@gagbo, I believe the issue was just that the Emacs plugin isn't providing a string_delimiter argument which was previously set by default. https://github.com/justinbarclay/parinfer-rust-mode/issues/70

gagbo commented 1 year ago

The I suppose the issue is stemming from here not parinfer-rust-mode.

The commits in the span you found seem to indicate that string_delimiters is supposed to have a default value of ", but the wrappers defined in this repo seem to ignore this part and force an empty list of delimiters instead on all exposed FFI bindings