Replace `--show-source` and `--no-show-source` with `--output_format=<full|concise>` #9687

snowsignal · 2024-01-29T19:21:34Z

Summary

--show-source and --no-show-source are now deprecated.
output-format supports two new variants, full and concise. text is now a deprecated variant, and any use of it is treated as the default serialization format.
--output-format now default to concise
In preview mode, --output-format defaults to full
--show-source will still set --output-format to full if the output format is not otherwise specified.
likewise, --no-show-source can override an output format that was set in a file-based configuration, though it will also be overridden by --output-format

Test Plan

A lot of tests were updated to use --output-format=full. Additional tests were added to ensure the correct deprecation warnings appeared, and that deprecated options behaved as intended.

zanieb · 2024-01-29T19:24:20Z

Woo! Maybe we should merge this into release/0.2.0 instead?

crates/ruff/src/printer.rs

crates/ruff/src/args.rs

docs/configuration.md

charliermarsh

This is great work -- really thorough and well-done. My only concern is: should we merge with concise as the default for now, so that it's not a change in behavior and just a deprecation?

crates/ruff/src/args.rs

crates/ruff/src/printer.rs

docs/configuration.md

snowsignal · 2024-01-29T19:59:01Z

My only concern is: should we merge with concise as the default for now, so that it's not a change in behavior and just a deprecation?

That's a good question! I thought it would be useful to solve two issues at once, but if it's too big of a change I'd be OK with splitting the changes into a separate PR. I'm curious to know your thoughts on this @zanieb.

zanieb · 2024-01-29T21:29:53Z

So I feel like the concern is that someone doesn't like the new output then is upset that the flag to toggle it back is deprecated? Perhaps it's just that there are too many changes in a single release?

My suggestion would be to gate the default with preview if feasible e.g. if --preview is passed then use --output-format=full and if not then use --output-format=concise. This would allow us to roll out the change in default separately in 0.3.0 instead.

Curious for @charliermarsh's thoughts though before we go changing things.

snowsignal · 2024-01-29T22:00:48Z

I added a new bundle of tests that ensure the deprecated options behave as expected.

So I feel like the concern is that someone doesn't like the new output then is upset that the flag to toggle it back is deprecated? Perhaps it's just that there are too many changes in a single release?

I understand that concern, but doesn't --output-format=concise solve that for them? Anyone using the deprecated flags will ideally get pointed in the right direction with the warning messages.

That being said, I'm not against gating this behind preview (though it would mean more work to split up these changes)

…the "text" variant, and deprecate the use of `--show-source`/`--no-show-source`, along with the respective configuration fields.

…converted to a default value

charliermarsh · 2024-01-29T22:22:05Z

Hmm yeah, introspecting a bit, I think my concern is that I wanted more user feedback before deciding that the full output format should be the default (which is a change from main, though was intended to be a settled decision per the relevant issue). So I was hoping to ship it to preview first to give us a chance to react to user feedback on the change. But I'm willing to give up on that, perhaps I'm being too cautious?

zanieb · 2024-01-29T23:07:11Z

I think we're pretty settled on wanting to move that direction as our default output because we can build a lot on top of it.

snowsignal · 2024-01-29T23:27:58Z

I've reverted us back to using concise as a default value. I'm fine with gating this behind preview until 0.3.0.

charliermarsh

Excellent! I'm still open to changing the default in v0.2.0, but we don't have to decide that to merge this PR.

crates/ruff/src/args.rs

crates/ruff/src/printer.rs

Fixes a regression in the ecosystem checks from #9687 which was causing them to run for multiple hours due to the size of the output. We need the concise format for comparisons. We should probably update the ecosystem checks to actually diff the full output in the future because that'd be nice.

…<full|concise>` (#9687) Fixes #7350 ## Summary * `--show-source` and `--no-show-source` are now deprecated. * `output-format` supports two new variants, `full` and `concise`. `text` is now a deprecated variant, and any use of it is treated as the default serialization format. * `--output-format` now default to `concise` * In preview mode, `--output-format` defaults to `full` * `--show-source` will still set `--output-format` to `full` if the output format is not otherwise specified. * likewise, `--no-show-source` can override an output format that was set in a file-based configuration, though it will also be overridden by `--output-format` ## Test Plan A lot of tests were updated to use `--output-format=full`. Additional tests were added to ensure the correct deprecation warnings appeared, and that deprecated options behaved as intended.

Fixes a regression in the ecosystem checks from #9687 which was causing them to run for multiple hours due to the size of the output. We need the concise format for comparisons. We should probably update the ecosystem checks to actually diff the full output in the future because that'd be nice.

…<full|concise>` (#9687) Fixes #7350 ## Summary * `--show-source` and `--no-show-source` are now deprecated. * `output-format` supports two new variants, `full` and `concise`. `text` is now a deprecated variant, and any use of it is treated as the default serialization format. * `--output-format` now default to `concise` * In preview mode, `--output-format` defaults to `full` * `--show-source` will still set `--output-format` to `full` if the output format is not otherwise specified. * likewise, `--no-show-source` can override an output format that was set in a file-based configuration, though it will also be overridden by `--output-format` ## Test Plan A lot of tests were updated to use `--output-format=full`. Additional tests were added to ensure the correct deprecation warnings appeared, and that deprecated options behaved as intended. # Conflicts: # crates/ruff/tests/integration_test.rs

Fixes a regression in the ecosystem checks from #9687 which was causing them to run for multiple hours due to the size of the output. We need the concise format for comparisons. We should probably update the ecosystem checks to actually diff the full output in the future because that'd be nice. # Conflicts: # python/ruff-ecosystem/ruff_ecosystem/projects.py

…<full|concise>` (#9687) Fixes #7350 ## Summary * `--show-source` and `--no-show-source` are now deprecated. * `output-format` supports two new variants, `full` and `concise`. `text` is now a deprecated variant, and any use of it is treated as the default serialization format. * `--output-format` now default to `concise` * In preview mode, `--output-format` defaults to `full` * `--show-source` will still set `--output-format` to `full` if the output format is not otherwise specified. * likewise, `--no-show-source` can override an output format that was set in a file-based configuration, though it will also be overridden by `--output-format` ## Test Plan A lot of tests were updated to use `--output-format=full`. Additional tests were added to ensure the correct deprecation warnings appeared, and that deprecated options behaved as intended. # Conflicts: # crates/ruff/tests/integration_test.rs

Fixes a regression in the ecosystem checks from #9687 which was causing them to run for multiple hours due to the size of the output. We need the concise format for comparisons. We should probably update the ecosystem checks to actually diff the full output in the future because that'd be nice. # Conflicts: # python/ruff-ecosystem/ruff_ecosystem/projects.py

AlexWaygood · 2024-02-01T20:10:46Z

crates/ruff/src/lib.rs

@@ -325,9 +321,14 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
        printer_flags,
    );

+    let preview = overrides.preview.unwrap_or_default().is_enabled();


Does this respect preview = true in configuration files? I think this might only respect --preview when it's set on the command line -- is that deliberate?

No that was not deliberate, good catch.

No worries! Was trying to pin down the build failures on #9599 after merging in main, and spotted this :)

Related changes in Ruff: astral-sh/ruff#9687

snowsignal added the cli Related to the command-line interface label Jan 29, 2024

snowsignal requested review from charliermarsh and zanieb January 29, 2024 19:21

snowsignal changed the title ~~Replace --show-source and --no-show-source with output_format=<full|concise>~~ Replace --show-source and --no-show-source with --output_format=<full|concise> Jan 29, 2024

zanieb reviewed Jan 29, 2024

View reviewed changes

crates/ruff/src/printer.rs Outdated Show resolved Hide resolved

zanieb reviewed Jan 29, 2024

View reviewed changes

crates/ruff/src/args.rs Outdated Show resolved Hide resolved

zanieb reviewed Jan 29, 2024

View reviewed changes

docs/configuration.md Outdated Show resolved Hide resolved

snowsignal changed the base branch from main to release/0.2.0 January 29, 2024 19:39

charliermarsh reviewed Jan 29, 2024

View reviewed changes

crates/ruff/src/args.rs Outdated Show resolved Hide resolved

crates/ruff/src/args.rs Show resolved Hide resolved

crates/ruff/src/printer.rs Outdated Show resolved Hide resolved

docs/configuration.md Outdated Show resolved Hide resolved

zanieb force-pushed the release/0.2.0 branch from 97afc70 to 2dea20f Compare January 29, 2024 21:01

snowsignal force-pushed the jane/cli/replace-show-sources-with-format branch 2 times, most recently from 49df927 to 9ba18cb Compare January 29, 2024 22:12

snowsignal added 5 commits January 29, 2024 14:13

Introduce "full" and "concise" options to output-format, deprecate …

2d0f7ee

…the "text" variant, and deprecate the use of `--show-source`/`--no-show-source`, along with the respective configuration fields.

Update tests and schema. A few issues have been fixed in the process.

80ec848

Address PR suggestions

6bce15a

Improve warning messages and ensure SerializationFormat::Text gets …

4a69e52

…converted to a default value

Add tests to ensure deprecated options display the proper warnings

71e7408

snowsignal force-pushed the jane/cli/replace-show-sources-with-format branch from 9ba18cb to 71e7408 Compare January 29, 2024 22:13

snowsignal requested a review from charliermarsh January 29, 2024 22:15

Fill in missing snapshots for the deprecation tests

672733c

Use concise as a default output format

8ebc7c7

snowsignal force-pushed the jane/cli/replace-show-sources-with-format branch from 1a6e0fe to 8ebc7c7 Compare January 29, 2024 23:23

Preview mode uses as the default output format

b14603d

snowsignal requested a review from zanieb January 29, 2024 23:37

charliermarsh approved these changes Jan 29, 2024

View reviewed changes

charliermarsh reviewed Jan 29, 2024

View reviewed changes

crates/ruff/src/args.rs Show resolved Hide resolved

charliermarsh reviewed Jan 29, 2024

View reviewed changes

crates/ruff/src/printer.rs Outdated Show resolved Hide resolved

snowsignal added 2 commits January 29, 2024 16:32

Address suggestions, add integration test to ensure preview mode uses

f761554

Derive Default for RuffCheck

e06a385

snowsignal force-pushed the jane/cli/replace-show-sources-with-format branch from bd27bb5 to e06a385 Compare January 30, 2024 00:45

snowsignal merged commit b244178 into release/0.2.0 Jan 30, 2024
16 of 17 checks passed

snowsignal deleted the jane/cli/replace-show-sources-with-format branch January 30, 2024 00:57

zanieb added the preview Related to preview mode features label Jan 30, 2024

This was referenced Jan 30, 2024

Release v0.2.0 #9680

Merged

Always request the concise output format during ecosystem checks #9708

Merged

AlexWaygood reviewed Feb 1, 2024

View reviewed changes

zanieb mentioned this pull request Feb 25, 2024

CLI: Show source context by default in ruff check #7349

Closed

hartwork added a commit to kevin1024/vcrpy that referenced this pull request Jun 30, 2024

pre-commit: Fix Ruff invocation for >=0.5.0

39a86ba

Related changes in Ruff: astral-sh/ruff#9687

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `--show-source` and `--no-show-source` with `--output_format=<full|concise>` #9687

Replace `--show-source` and `--no-show-source` with `--output_format=<full|concise>` #9687

snowsignal commented Jan 29, 2024 •

edited

Loading

zanieb commented Jan 29, 2024 •

edited

Loading

charliermarsh left a comment

snowsignal commented Jan 29, 2024

zanieb commented Jan 29, 2024 •

edited

Loading

snowsignal commented Jan 29, 2024

charliermarsh commented Jan 29, 2024

zanieb commented Jan 29, 2024

snowsignal commented Jan 29, 2024

charliermarsh left a comment

AlexWaygood Feb 1, 2024

snowsignal Feb 1, 2024

AlexWaygood Feb 1, 2024

Replace --show-source and --no-show-source with --output_format=<full|concise> #9687

Replace --show-source and --no-show-source with --output_format=<full|concise> #9687

Conversation

snowsignal commented Jan 29, 2024 • edited Loading

Summary

Test Plan

zanieb commented Jan 29, 2024 • edited Loading

charliermarsh left a comment

Choose a reason for hiding this comment

snowsignal commented Jan 29, 2024

zanieb commented Jan 29, 2024 • edited Loading

snowsignal commented Jan 29, 2024

charliermarsh commented Jan 29, 2024

zanieb commented Jan 29, 2024

snowsignal commented Jan 29, 2024

charliermarsh left a comment

Choose a reason for hiding this comment

AlexWaygood Feb 1, 2024

Choose a reason for hiding this comment

snowsignal Feb 1, 2024

Choose a reason for hiding this comment

AlexWaygood Feb 1, 2024

Choose a reason for hiding this comment

Replace `--show-source` and `--no-show-source` with `--output_format=<full|concise>` #9687

Replace `--show-source` and `--no-show-source` with `--output_format=<full|concise>` #9687

snowsignal commented Jan 29, 2024 •

edited

Loading

zanieb commented Jan 29, 2024 •

edited

Loading

zanieb commented Jan 29, 2024 •

edited

Loading