rust-lang / rust-mode

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

Make more easily comparable with rustic fork #393

Closed tarsius closed 4 years ago

tarsius commented 4 years ago

A few months ago it looked like I was going to learn Rust and so I looked for the best Emacs major-mode. (Actually learning Rust got delayed, but I still plan on doing it.) I found this rust-mode and the rustic fork by @brotzeit.

I think that fork was a bit unfortunate. What @brotzeit mostly did was to add extra functionality but he also refactored the existing essential major-mode/syntax/... parts. Many of these changes were rather superficial, but because I did not backport them immediately and made major rearrangements it became very hard to compare the respective bits from the two implementations.

These commits, which I wrote five months ago for the most part, try to fix that. I already got a lot of cleanup merged into rustic before, what these commits here to is to arrange rust-mode.el the same way rustic.el has been rearranged before. Other commits port doc-string improvements, other cleanup and the addition of outline-minor-mode-compatible section headings.

After applying these changes you can copy rustic.el from the other repository, rename it to rust-mode.el and also do s/rustic/rust inside and then you get pretty useful diff. (I do not think that all rearrangement is an improvement or even just necessary. Some are, but others don't really make a difference either way.)

My hope is that I can get you guys to cooperate some more, so that eventually there won't be two major-modes anymore. I think it makes sense for there two be two projects, a battery-included thing like rustic and a just-the-major-mode rust-mode, but it would be great if rust could just use the rust-mode.el from rust-mode.

I have written these commits and a lot of cleanup to rustic that has already been merged, but I think that from here you will have to pick it up and make the fork a thing of the past.

Thanks for considering this proposal, and lots of fun with the commits!

rust-highfive commented 4 years ago

r? @mookid

(rust_highfive has picked a reviewer for you, use r? to override)

mookid commented 4 years ago

Some tests fail. I suggest to make sure that tests pass at each step of the refactoring, as it is easy to omit some code.

mookid commented 4 years ago

I made a few comments concerning a couple details (I reviewed commit by commit). I am ok to merge when tests pass at each step of the refactoring, and I believe that this goes in the right direction.

However, I don't really know what can be achieved on the mid term; I miss time to work on this, and I prefer to spend that time improving the code here rather than hypothetically converging with the fork. And not many other people want to work on rust-mode.

tarsius commented 4 years ago

Great! I must admit I did not test after rebasing this a few times on top of new changes. But knowing that you intend to merge, the motivation is back and I will give this the necessary final polish. Stay tuned.

tarsius commented 4 years ago

Tests pass now. The only problem I found was those few functions that were placed after they are used while setting the initial values of some variables; which you pointed out inline.

tarsius commented 4 years ago

However, I don't really know what can be achieved on the mid term; I miss time to work on this, and I prefer to spend that time improving the code here rather than hypothetically converging with the fork. And not many other people want to work on rust-mode.

No problem. I wanted to land this now because dealing because resolving conflicts when rebasing onto incoming changes became a bit annoying. I probably won't contribute anything soon, but eventually I probably will return. ;D

brotzeit commented 4 years ago

@tarsius thanks for your work. I'm absolutely in favor of reducing the maintenance effort for both packages. However I'm still not sure how we want to treat similiar functions that exist in both packages. For example rust-format-buffer in rust-mode works differently than the equivalent in rustic. Do you have any idea how we can solve this ?

mookid commented 4 years ago

Looks good to me, merging tomorrow if nobody screams. Thanks for the work!

tarsius commented 4 years ago

/all Cheers!

@brotzeit Those can coexist. Maybe you have to add some safety wheels to prevent that both of them are used at them same time, but I don't see a problem here.

brotzeit commented 4 years ago

Hmmm. Ok I will try to do it incrementelly somehow.