Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_cli): expose the --verbose flag to the CLI #3812

Merged
merged 4 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions crates/rome_cli/src/commands/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const CHECK: Markup = markup! {
"<Dim>"--apply"</Dim>" Apply safe fixes
"<Dim>"--apply-suggested"</Dim>" Apply safe and suggested fixes
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 20)
"<Dim>"--verbose"</Dim>" Print additional verbose advices on diagnostics
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the --verbose argument should be global. Much like --colors.

Any command can emit diagnostics, hence any diagnostic can have, theoretically, verbose advice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well at the moment only check, ci and format can print diagnostics since Termination hasn't been migrated to the diagnostic system yet. But yes eventually I think this will become a global flag for the CLI

"
};

Expand All @@ -63,7 +64,8 @@ const CI: Markup = markup! {
"<Emphasis>"OPTIONS:"</Emphasis>"
"<Dim>"--formatter-enabled"</Dim>" Allow to enable or disable the formatter check. (default: true)
"<Dim>"--linter-enabled"</Dim>" Allow to enable or disable the linter check. (default: true)
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 50)"
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 50)
"<Dim>"--verbose"</Dim>" Print additional verbose advices on diagnostics"
{FORMAT_OPTIONS}
};

Expand All @@ -78,7 +80,8 @@ const FORMAT: Markup = markup! {
"<Emphasis>"OPTIONS:"</Emphasis>"
"<Dim>"--write"</Dim>" Edit the files in place (beware!) instead of printing the diff to the console
"<Dim>"--skip-errors"</Dim>" Skip over files containing syntax errors instead of emitting an error diagnostic.
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 50)"
"<Dim>"--max-diagnostics"</Dim>" Cap the amount of diagnostics displayed (default: 50)
"<Dim>"--verbose"</Dim>" Print additional verbose advices on diagnostics"
{FORMAT_OPTIONS}
""<Dim>"--stdin-file-path <string>"</Dim>" A file name with its extension to pass when reading from standard in, e.g. echo 'let a;' | rome format --stdin-file-path file.js
"
Expand Down
16 changes: 11 additions & 5 deletions crates/rome_cli/src/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ impl fmt::Display for CheckResult {
pub(crate) fn traverse(execution: Execution, mut session: CliSession) -> Result<(), Termination> {
init_thread_pool();

let verbose = session.args.contains("--verbose");

// Check that at least one input file / directory was specified in the command line
let mut inputs = vec![];

Expand Down Expand Up @@ -116,6 +118,7 @@ pub(crate) fn traverse(execution: Execution, mut session: CliSession) -> Result<
remaining_diagnostics: &remaining_diagnostics,
errors: &mut errors,
report: &mut report,
verbose,
});
})
.expect("failed to spawn console thread");
Expand Down Expand Up @@ -262,6 +265,8 @@ struct ProcessMessagesOptions<'ctx> {
/// Mutable handle to a [Report] instance the console thread should write
/// stats into
report: &'ctx mut Report,
/// Whether the console thread should print diagnostics in verbose mode
verbose: bool,
}

#[derive(Debug, Diagnostic)]
Expand Down Expand Up @@ -328,6 +333,7 @@ fn process_messages(options: ProcessMessagesOptions) {
remaining_diagnostics,
errors,
report,
verbose,
} = options;

let mut paths = HashMap::new();
Expand Down Expand Up @@ -412,7 +418,7 @@ fn process_messages(options: ProcessMessagesOptions) {
if mode.should_report_to_terminal() {
if should_print {
console.error(markup! {
{PrintDiagnostic(&err)}
{if verbose { PrintDiagnostic::verbose(&err) } else { PrintDiagnostic::simple(&err) }}
});
}
} else {
Expand Down Expand Up @@ -454,7 +460,7 @@ fn process_messages(options: ProcessMessagesOptions) {

let diag = diag.with_file_path(&name).with_file_source_code(&content);
console.error(markup! {
{PrintDiagnostic(&diag)}
{if verbose { PrintDiagnostic::verbose(&diag) } else { PrintDiagnostic::simple(&diag) }}
});
}
} else {
Expand All @@ -480,7 +486,7 @@ fn process_messages(options: ProcessMessagesOptions) {
let diag =
diag.with_file_path(&name).with_file_source_code(&content);
console.error(markup! {
{PrintDiagnostic(&diag)}
{if verbose { PrintDiagnostic::verbose(&diag) } else { PrintDiagnostic::simple(&diag) }}
});
}
} else {
Expand Down Expand Up @@ -530,7 +536,7 @@ fn process_messages(options: ProcessMessagesOptions) {
};

console.error(markup! {
{PrintDiagnostic(&diag)}
{if verbose { PrintDiagnostic::verbose(&diag) } else { PrintDiagnostic::simple(&diag) }}
});
} else {
let diag = FormatDiffDiagnostic {
Expand All @@ -542,7 +548,7 @@ fn process_messages(options: ProcessMessagesOptions) {
};

console.error(markup! {
{PrintDiagnostic(&diag)}
{if verbose { PrintDiagnostic::verbose(&diag) } else { PrintDiagnostic::simple(&diag) }}
});
}
}
Expand Down
32 changes: 32 additions & 0 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,3 +1086,35 @@ a == b;",
result,
));
}

#[test]
fn print_verbose() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let file_path = Path::new("check.js");
fs.insert(file_path.into(), LINT_ERROR.as_bytes());

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![
OsString::from("check"),
OsString::from("--verbose"),
file_path.as_os_str().into(),
]),
);

match result {
Err(Termination::CheckError) => {}
_ => panic!("run_cli returned {result:?} for a failed CI check, expected an error"),
}

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"print_verbose",
fs,
console,
result,
));
}
32 changes: 32 additions & 0 deletions crates/rome_cli/tests/commands/ci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,3 +633,35 @@ fn max_diagnostics() {

assert_eq!(diagnostic_count, 10);
}

#[test]
fn print_verbose() {
let mut fs = MemoryFileSystem::default();

let file_path = Path::new("ci.js");
fs.insert(file_path.into(), LINT_ERROR.as_bytes());

let mut console = BufferConsole::default();
let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![
OsString::from("ci"),
OsString::from("--verbose"),
file_path.as_os_str().into(),
]),
);

match &result {
Err(Termination::CheckError) => {}
_ => panic!("run_cli returned {result:?} for a failed CI check, expected an error"),
}

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"print_verbose",
fs,
console,
result,
));
}
29 changes: 29 additions & 0 deletions crates/rome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1414,3 +1414,32 @@ fn no_supported_file_found() {
result,
));
}

#[test]
fn print_verbose() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let file_path = Path::new("format.js");
fs.insert(file_path.into(), UNFORMATTED.as_bytes());

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![
OsString::from("format"),
OsString::from("--verbose"),
file_path.as_os_str().into(),
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"print_verbose",
fs,
console,
result,
));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
## `check.js`

```js
for(;true;);

```

# Termination Message

```block
some errors were emitted while running checks
```

# Emitted Messages

```block
check.js:1:1 lint/correctness/useWhile FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Use while loops instead of for loops.

> 1 │ for(;true;);
│ ^^^^^^^^^^^
2 │

i Suggested fix: Use a while loop

1 │ - for(;true;);
1 │ + while·(true);
2 2 │


```

```block
check.js:1:1 lint/style/useBlockStatements FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Block statements are preferred in this position.

> 1 │ for(;true;);
│ ^^^^^^^^^^^^
2 │

i Suggested fix: Wrap the statement with a `JsBlockStatement`

1 │ - for(;true;);
1 │ + for(;true;)·{}
2 2 │


```


Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
## `ci.js`

```js
for(;true;);

```

# Termination Message

```block
some errors were emitted while running checks
```

# Emitted Messages

```block
ci.js:1:1 lint/correctness/useWhile FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Use while loops instead of for loops.

> 1 │ for(;true;);
│ ^^^^^^^^^^^
2 │

i Suggested fix: Use a while loop

1 │ - for(;true;);
1 │ + while·(true);
2 2 │


```

```block
ci.js:1:1 lint/style/useBlockStatements FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× Block statements are preferred in this position.

> 1 │ for(;true;);
│ ^^^^^^^^^^^^
2 │

i Suggested fix: Wrap the statement with a `JsBlockStatement`

1 │ - for(;true;);
1 │ + for(;true;)·{}
2 2 │


```


Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
## `format.js`

```js
statement( )
```

# Emitted Messages

```block
format.js format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i Formatter would have printed the following content:

1 │ - ··statement(··)··
1 │ + statement();
2 │ +


```


2 changes: 1 addition & 1 deletion crates/rome_diagnostics/examples/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@ pub fn main() {
},
};

EnvConsole::default().error(markup!({ PrintDiagnostic(&diag) }));
EnvConsole::default().error(markup!({ PrintDiagnostic::verbose(&diag) }));
}
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/examples/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,5 @@ pub fn main() {
},
};

EnvConsole::default().error(markup!({ PrintDiagnostic(&diag) }));
EnvConsole::default().error(markup!({ PrintDiagnostic::verbose(&diag) }));
}
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/examples/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,5 @@ console.log(FOO);";
},
};

EnvConsole::default().error(markup!({ PrintDiagnostic(&diag) }));
EnvConsole::default().error(markup!({ PrintDiagnostic::verbose(&diag) }));
}
2 changes: 1 addition & 1 deletion crates/rome_diagnostics/examples/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ fn from_str(input: &str) -> Result<serde_json::Value> {

pub fn main() {
if let Err(err) = from_str("{\"syntax_error\"") {
EnvConsole::default().error(markup!({ PrintDiagnostic(&err) }));
EnvConsole::default().error(markup!({ PrintDiagnostic::verbose(&err) }));
};
}
Loading