Refactor for 2.0 #4168
Refactor for 2.0 #4168
Conversation
| /// Client-preference for coloured output. | ||
| #[derive(Debug, Clone, Copy, PartialEq)] | ||
| pub enum Color { | ||
| /// Always use color, whether it is a piped or terminal output | ||
| Always, | ||
| /// Never use color | ||
| Never, | ||
| /// Automatically use color, if supported by terminal | ||
| Auto, | ||
| } |
There was a problem hiding this comment.
Are there plans to re-add support for color as a config option later on? If not it may be worth getting rid of the Always and Never variants, looks like they aren't used.
There was a problem hiding this comment.
No, there are not. We may add it back to the command-line interface. I would prefer to keep these variants as a part of the public API for rustfmt-as-a-library.
| // We missed some comments. Warn and keep the original text. | ||
| if context.config.error_on_unformatted() { | ||
| context.report.append( | ||
| context.parse_sess.span_to_filename(span), | ||
| vec![FormattingError::from_span( | ||
| span, | ||
| &context.parse_sess, | ||
| ErrorKind::LostComment, | ||
| )], | ||
| ); | ||
| } |
There was a problem hiding this comment.
This error message is for rustfmt developers, so end-users should not see them. I guess we can keep this as a log message instead.
| pub fn newline_style(&self) -> NewlineStyle { | ||
| self.newline_style.clone() | ||
| } | ||
|
|
||
| pub fn original_text(&self) -> Option<&str> { | ||
| self.original_snippet.as_ref().map(|s| s.as_str()) | ||
| } | ||
|
|
||
| pub fn formatted_text(&self) -> &str { |
There was a problem hiding this comment.
Why not make these crate public too?
There was a problem hiding this comment.
These methods are a part of the public API of rustfmt-as-a-library.
| if original_format_result.original_snippet.is_none() { | ||
| original_format_result.original_snippet = format_result.original_snippet; | ||
| } |
There was a problem hiding this comment.
nit: why not just perform the assignment?
| for (file, errors) in errors_by_file { | ||
| for error in errors { | ||
| for (file, errors) in self.report.format_result_as_rc().borrow().iter() { | ||
| for error in errors.errors_excluding_macro() { |
There was a problem hiding this comment.
Why are macro errors excluded?
There was a problem hiding this comment.
Macro-related errors are filtered because they are for internal use, and we don't want users to see them.
topecongiro
force-pushed
the
rearch
branch
2 times, most recently
from
2ff3afd to
1265561
Compare
|
I can set aside some time this weekend to read through this one in more detail, but generally good with the changes if you want to go ahead and merge as well |
| use crate::{ | ||
| formatting::report::FormatReport, | ||
| result::{ErrorKind, FormatError}, | ||
| }; | ||
| use annotate_snippets::display_list::DisplayList; |
There was a problem hiding this comment.
Seems there's a decent amount of changes with the latest version of annotate_snippets and #4161 was opened to apply those changes to rustfmt. Not sure what the best merge sequencing would be between these two PRs
| @@ -1,4 +1,5 @@ | |||
| // rustfmt-config: issue-3933.toml | |||
| // rustfmt-recursive: true | |||
|
backport not needed (doesn't look to me like it's feasible) |
This PR makes extensive internal/public changes to rustfmt codebase. It touches and modifies many files, which makes it hard to review this PR. I will try to follow up with a detailed explanation, but please feel free to ask questions if anything is unclear.
This PR specifically does the following:
Config.