search

rust-lang / rust-mode

Emacs configuration for Rust
Apache License 2.0
1.11k stars 179 forks source link

provide alternative rust-mode that derives from rust-ts-mode #482

Closed brotzeit closed 7 months ago

thblt commented 1 year ago

I was just looking into a way to complement the new rust-ts-mode with rust-mode features. Wouldn't it be nicer to separate i those features as a minor mode that could be hooked to rust-ts-mode?

brotzeit commented 1 year ago

If rust-mode was a new package that's supposed to add features to rust-ts-mode it would make sense to use a minor mode. I think there is no real downside to using a derived mode and this way we don't have to apply any additional changes. We can still change this once treesitter is the standard and the old code is deprecated. For now it seems to be the more convenient solution.

But maybe I'm wrong. Other opinions ? @jimblandy @psibi

thblt commented 1 year ago

Ha, I see your point, and backward compatibility is indeed a nice thing :)

Maybe it would be nice if the control variable defaulted to t whenever rust-ts-mode is available so the transition will be perfectly transparent:

(defcustom rust-mode-treesitter-derive
  (and (fboundp 'treesit-language-available-p)
       (treesit-language-available-p 'rust))
…
brotzeit commented 1 year ago

I thought about it but the ts mode applies different syntax highlighting. Users will probably complain that we just change the default. Many rust-mode users only want very basic features. I know those people probably won't use emacs master, but I would wait.

jimblandy commented 1 year ago

I think we should always feel free to make substantial improvements even when they change behavior. If treesitter is available, reliable, and really provides the improvements it claims to, we should just use it.

thblt commented 1 year ago

Is anything blocking this feature being merged? AFAICS it should be a noop until explicitly enabled.

creese commented 11 months ago

Can this be merged?

uncomfyhalomacro commented 11 months ago

hello. will this be merged or are there any more to be done first? thank you!

psibi commented 7 months ago

I have done a minor change to this PR to make it work for my setup (Emacs 29.1). I can confirm that it works with my basic testing. I'm planning to test drive this PR for around a week to see if I face any issues and merge it if I see no issues.

If others can test drive this PR meanwhile, that will be helpful. An easy way to test this is setting the following variable:

(setq rust-mode-treesitter-derive t)

And in your rust code buffer, check that tree sitter based functions is working (Eg: treesit-end-of-defun).