eraserhd / parinfer-rust

A Rust port of parinfer.
ISC License
546 stars 42 forks source link

Mishandling semicolon inside vertical bar literal #115

Open ylixir opened 3 years ago

ylixir commented 3 years ago

Parinfer thinks that semicolons between vertical bars is a comment (it's not). At least in common lisp, racket, and lumen this is wrong

(define |;| 5)
(print |;|)

is changed to

(define |);| 5)
(print |);|)

which is wrong.

reproductions: https://onecompiler.com/racket/3xjvghuay https://onecompiler.com/commonlisp/3xjvgwzbj

this is burning me when | is being used to quote literal code in a transpiler: https://github.com/sctb/lumen/blob/master/test.l#L202

jul1u5 commented 2 years ago

Seems related to #77 — enabling lisp_vline_symbols option fixes the issue.

However, #78 only implements this for (neo)vim plugin, so currently, there's no support for this in Emacs: https://github.com/eraserhd/parinfer-rust/blob/9e41222b7bc8930e24c411c1b4bc48715975ed17/src/emacs_wrapper.rs#L160

(I haven't checked, but I think kakoune plugin doesn't implement this either)

Potential Fix for Emacs

  1. Add lisp_vline_symbols parameter for new_options (and make_option) in parinfer-rust: https://github.com/eraserhd/parinfer-rust/blob/9e41222b7bc8930e24c411c1b4bc48715975ed17/src/emacs_wrapper.rs#L141-L147

  2. Pass additional argument for lisp_vline_symbols in parinfer-rust-mode: https://github.com/justinbarclay/parinfer-rust-mode/blob/c2c1bbec6cc7dad4f546868aa07609b8d58a78f8/parinfer-rust-mode.el#L346-L351

But this will be a breaking change: the new version of parinfer-rust will not be compatible with the current parinfer-rust-mode and vice versa.

Vim plugin doesn't have this problem since it communicates with parinfer-rust via JSON and sends options as a dictionary: https://github.com/eraserhd/parinfer-rust/blob/211f72e32cccbc728a8346f665c0a0b52c01c1e4/plugin/parinfer.vim#L213-L225 So it doesn't need to pass all the options.

Perhaps JSON communication could be used for Emacs mode too? Or is there a way to pass a dictionary/map from Elisp to Rust directly?

@eraserhd @justinbarclay, what do you think?