Skip to content

Remove output format text and use format full by default#12010

Merged
MichaReiser merged 3 commits intoruff-0.5from
output-format-full-by-default
Jun 26, 2024
Merged

Remove output format text and use format full by default#12010
MichaReiser merged 3 commits intoruff-0.5from
output-format-full-by-default

Conversation

@MichaReiser
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser commented Jun 24, 2024

Summary

This PR removes support for the output format text in favor of concise and makes full the new default (it was already the default for preview mode). The OutputFormat::Text is still defined in code so that we can error when it's being used.

Resolves #7349

Test Plan

I tested that using --output-format on the CLI errors:

❯ cargo run --bin ruff -- check --output-format text
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s
     Running `target/debug/ruff check --output-format text`
ruff failed
  Cause: `--output-format=text` is deprecated. Use `--output-format=full` or `--output-format=concise` instead.

❯ cargo run --bin ruff -- check --config="output-format='text'"
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s
     Running `target/debug/ruff check '--config=output-format='\''text'\'''`
ruff failed
  Cause: The Setting `output_format=text` has been deprecated. Update your `--config` CLI arguments to use `output-format="concise"` or  `output-format="full"` instead.

I verified that using output-format="text" in the settings errors

❯ ../ruff/target/debug/ruff check test.py
ruff failed
  Cause: The Setting `output_format=text` has been deprecated. Update `pyproject.toml` to use `output-format="concise"` or  `output-format="full"` instead.
  • I verified that using concise in the settings or on the CLI works as expected
  • I verified that ruff defaults to full when no output-format is specified on the CLI or in the settings.

@MichaReiser MichaReiser changed the title Remove output format text and use format full by defualt Remove output format text and use format full by default Jun 24, 2024
@MichaReiser MichaReiser changed the base branch from main to ruff-0.5 June 24, 2024 09:04
@MichaReiser MichaReiser requested review from snowsignal and zanieb June 24, 2024 09:05
@MichaReiser MichaReiser added breaking Breaking API change configuration Related to settings and configuration cli Related to the command-line interface labels Jun 24, 2024
Comment on lines +1301 to +1223
|
|
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This looks like a bug but it isn't. Or, it is a bug in the test rule that always emits an empty range instead of emitting an actual range.

@MichaReiser MichaReiser added this to the v0.5.0 milestone Jun 24, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 24, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Jun 24, 2024

I'm a little worried about making this change when the full format is half-complete (#7352)

@MichaReiser
Copy link
Copy Markdown
Member Author

@zanieb what's the part that you're worried about?

I think showing the code frame for diagnostics is already an improvement by itself worth shipping and the format name full is generic enough to allow us to also show fixes in a future version.

@zanieb
Copy link
Copy Markdown
Member

zanieb commented Jun 24, 2024

It just doesn't feel as useful over the concise output without the fixes being shown (seen feedback from one user about this). Oh well though.

@MichaReiser
Copy link
Copy Markdown
Member Author

I personally find the source text in the rust compiler extremely useful. I don't think I ever look at the line number to get context on where I need to fix something.

Copy link
Copy Markdown
Contributor

@snowsignal snowsignal left a comment

Choose a reason for hiding this comment

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

LGTM - it might be nice to have a small test plan :)

@MichaReiser MichaReiser force-pushed the output-format-full-by-default branch 2 times, most recently from 7aa60a4 to 9bb1ccc Compare June 25, 2024 08:15
@MichaReiser MichaReiser marked this pull request as ready for review June 25, 2024 08:15
@MichaReiser
Copy link
Copy Markdown
Member Author

I updated the PR to keep the Text option for now but I made the usage of Text a hard error.

@MichaReiser MichaReiser force-pushed the output-format-full-by-default branch from 9bb1ccc to b8d1ba6 Compare June 26, 2024 06:54
@MichaReiser MichaReiser merged commit 0501afd into ruff-0.5 Jun 26, 2024
@MichaReiser MichaReiser deleted the output-format-full-by-default branch June 26, 2024 07:40
This was referenced Jun 26, 2024
MichaReiser added a commit that referenced this pull request Jun 27, 2024
MichaReiser added a commit that referenced this pull request Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking API change cli Related to the command-line interface configuration Related to settings and configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants