Rustfmt libterm #28907
Rustfmt libterm #28907
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| }, | ||
| None => return Err("bad param number".to_owned()), | ||
| }] | ||
| .clone()); |
There was a problem hiding this comment.
|
r+, you'll need to add |
|
Cool bananas, will follow up on that later today so those tests can get passing. |
| } | ||
| } else { return Err("stack is empty".to_owned()) }, | ||
| } else { | ||
| return Err("stack is empty".to_owned()) |
There was a problem hiding this comment.
rustfmt now appends these statements with semicolons; may be worthwhile to rerun this?
|
Cheers for the heads up. I will look at rerunning rustfmt and updating this PR. |
|
I've merged master back into this branch, removed the rustfmt_skips, and rerun rustfmt over the last commit in this branch. I don't see any particularly odd changes, and the statics are formatted without breaking the line limit. However it does result in some very long statics. I'd personally prefer the statics to remain truncated for the sake of avoiding hundreds of lines of them. Would be interested in other's thoughts on the matter. |
|
It looks like this latest batch of formatting causes a compilation failure in compiled.rs, I will investigate. |
|
Not sure if I've bungled this a bit by attempting to merge master with the already made formatting. So I'm just going to reset this branch to master and repeat the formatting to make sure problems have not been introduced, and to make the work flow easier to follow. |
SingingTree
force-pushed
the
rustfmt_libterm
branch
from
bcc446e to
ad2a0c5
Compare
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
I think the removal of the semi-colon here causes issues.
|
☔ The latest upstream changes (presumably #29546) made this pull request unmergeable. Please resolve the merge conflicts. |
SingingTree
force-pushed
the
rustfmt_libterm
branch
from
2330c16 to
7d54c32
Compare
|
Updated to work with upstream changes. Changes made reflect rustfmt's changes, aside from the skips in compiled.rs to avoid splitting the statics over many more lines. |
|
@bors: r+ |
|
📌 Commit 7d54c32 has been approved by |
Hey hey, This is the result of running rustfmt over the libterm module. The first commit reflects the unaltered changes from rustfmt, and the commit message contains some notes on areas where I thought rustfmt had behaved strangely. The second commit attempts to fix the strange areas from the first commit. Clarification edit: there are still some areas where I think rustfmt has made changes which may merit discussion (one is noted in the comments below). My second commit only undoes the changes that I figured would not warrant discussion (based on my opinion of the style, which is of course subjective). r? @nrc
Hey hey,
This is the result of running rustfmt over the libterm module. The first commit reflects the unaltered changes from rustfmt, and the commit message contains some notes on areas where I thought rustfmt had behaved strangely. The second commit attempts to fix the strange areas from the first commit.
Clarification edit: there are still some areas where I think rustfmt has made changes which may merit discussion (one is noted in the comments below). My second commit only undoes the changes that I figured would not warrant discussion (based on my opinion of the style, which is of course subjective).
r? @nrc