-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat: --check
mode
#16
Conversation
@@ -32,6 +36,7 @@ fn main() { | |||
max_width: args.max_width, | |||
tab_spaces: args.tab_spaces, | |||
attr_value_brace_style: AttributeValueBraceStyle::WhenRequired, | |||
allow_changes: !args.check, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep this outside of the formatting settings. These settings are describing how the code should be formatted, allow_changes
is not a formatting rule in itself, if that makes sense?
Hey, thanks for working on this! I propose a slightly different approach, what if we introduce the concept of trait Emitter {
type Result = ();
fn emit_formatted_file(
&mut self,
output: &mut dyn Write,
formatted_file: FormattedFile<'_>,
) -> Result<Self::Result, io::Error>; where a formatted file would be: struct FormattedFile<'a> {
filename: &'a PathBuf,
original_text: &'a str,
formatted_text: &'a str,
} We could then have struct DiffEmitterResult {
has_diff: bool
}
struct DiffEmitter;
impl DiffEmitter {
type Result = DiffEmitterResult
...
} And based on the result ( Thoughts? |
Would you like to pick this up? Alternatively I could implement the Or if you dislike the whole idea about emitters, let me know, I am open for feedback ;) |
Thanks for the input! Sorry I haven't checked on this in a while. I like the callout of pulling I'd be happy to follow your lead on implementing a second |
I see what you mean. What if we see it as 3 different phases:
Maybe we could translate this into:
So that: struct OutputFormatResult;
trait OutputFormat {
fn output_formatted_file(
&mut self,
output: &mut dyn Write, // this could be a file or stdout
formatted_file: FormattedFile<'_>,
) -> Result<ExitCode, io::Error>;
}
struct PlainOutputFormat;
impl OutputFormat for PlainOutputFormat {
fn output_formatted_file(
&mut self,
output: &mut dyn Write, // this could be a file or stdout
formatted_file: FormattedFile<'_>,
) -> Result<(), io::Error> {
output.write(formatted_file.formatted_text);
ExitCode::Success
}
}
struct DiffOutputFormat;
// your implementation here
Ok will give you a sign when PR is ready, will assign you as reviewer : ) |
a58efee
to
0de7655
Compare
Any updates on this one? It'd be really nice to have a check for CI. |
In the meanwhile, you can use |
superseded by #72 |
Implements #12
Now, by adding a
--check
flag, the program will exit unsuccessfully if it should have made changes to the files. If it didn't need to make any changes, then it will exit successfully, as usual.This main check happens within the
source_file::format_file_source
function, however similar logic seems to be insource_file::format_expr_source
. Do we need to copy this logic into that function as well?