rust-lang / rust-mode

Emacs configuration for Rust
Apache License 2.0
1.1k stars 178 forks source link

In emacs >= 26.2, use replace-buffer-contents after formatting #371

Closed tspiteri closed 4 years ago

tspiteri commented 4 years ago

This saves the position and the markers in the buffer, so there is no need to save positions of direct and indirect buffers and windows. This also plays better with e.g. grep, as otherwise all grep markers would be destroyed when replacing the contents of the buffer with copy-to-buffer.

While replace-buffer-contents was added in Emacs 26.1, it was broken for non-ASCII contents and would corrupt the buffer contents. This was fixed in 26.2, so we require version 26.2 for this instead of just relying on fboundp (which still has to be used for byte compilation on Emacs < 26).

rust-highfive commented 4 years ago

r? @mookid

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

tspiteri commented 4 years ago

Fixes #260. Fixed #350. (Only for emacs version >= 26.2, but I think that's reasonable.)

mookid commented 4 years ago

Thanks for the investigation!

Alternatively to that conditional code, how costly (in terms of lines of code) would it be to backport the fixes made in upstream emacs to make that work? That would avoid the conditionals everywhere.

tspiteri commented 4 years ago

The replace-buffer-contents function was new in 26.1, and it is in C not in elisp, so I don't think it can be backported. And the essential 26.2 fix is also of course in the C code and cannot be backported. Porting the functions to elisp would be very costly I think; both in the sense that it would be a major undertaking to develop, and in the sense that it would have a large performance drop in a function that already can be very heavy if the formatting does a lot of changes.

mookid commented 4 years ago

The replace-buffer-contents function was new in 26.1, and it is in C not in elisp,

Ok, I did not think about the case of a fix in the C code. Thanks for the info.