Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Feb 24, 2024

Conversation

brotzeit
Copy link
Contributor

No description provided.

@thblt
Copy link

thblt commented Dec 27, 2022

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
Copy link
Contributor Author

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
Copy link

thblt commented Dec 28, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link

thblt commented Jul 14, 2023

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

@creese
Copy link

creese commented Oct 25, 2023

Can this be merged?

@uncomfyhalomacro
Copy link

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

@psibi
Copy link
Member

psibi commented Feb 17, 2024

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).

@psibi psibi merged commit bff7bd3 into master Feb 24, 2024
14 of 15 checks passed
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.

6 participants