Skip to content

use compilation buffer for rustfmt errors #246

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

Closed
wants to merge 1 commit into from

Conversation

brotzeit
Copy link
Contributor

@brotzeit brotzeit commented Nov 6, 2017

No description provided.

@tromey
Copy link
Contributor

tromey commented Nov 8, 2017

I like this idea, but I wonder about removing the "exit code 3" handling. IIRC that is there because sometimes rustfmt can't format specific lines -- so currently the changes are applied and a warning is given, but with this change, the changes won't be applied.

@brotzeit
Copy link
Contributor Author

brotzeit commented Nov 9, 2017

I forgot about that problem. That was actually the reason I closed a previous PR.
The updated solution is not very pretty but I haven't seen any other error with exit code 3 than "line exceeded maximum width".

I will use this solution, but I have no problem with changing it again if anybody has a better idea. But IMO this is much more convenient.

@brotzeit brotzeit force-pushed the master branch 2 times, most recently from 4355af7 to 174eb8b Compare November 9, 2017 17:34
(with-current-buffer (get-buffer-create "*rustfmt*")
(erase-buffer)
(insert-buffer-substring buf)
(let* ((tmpf (make-temp-file "rustfmt"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why this code previously used a temporary file? I wonder if there was some issue with using stdin.

(setq buf-string (buffer-string)))
(goto-char (point-min))
(save-excursion
(while (re-search-forward buf-string nil t)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this loop is for. Could you add a comment explaining it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the actual file contents from the compilation buffer, but you're right that can be done better. IMO rustfmt shouldn't format files if there are any kind of errors. I mean I have to take care of them anyway as I don't want an error message every time I use rustfmt. Hopefully that will be changed when it's more mature.

I can understand if you don't want to merge it, as it won't work correctly in every situation. I can try to make this more sound, but I can also wait until there's an improvement on the rustfmt side and give it another try.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know nothing, but I'm curious why we can't use compilation-start?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think compilation-start doesn't allow piping through stdin.

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.

3 participants