Skip to content

Rename tab-size to indent-width #8082

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.

Sign up for GitHub

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

Jump to bottom
Merged
charliermarsh merged 1 commit into from
Oct 24, 2023
Merged

Rename tab-size to indent-width #8082

charliermarsh merged 1 commit into from
Oct 24, 2023

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Oct 20, 2023 •
edited
Loading

Summary

This PR renames the tab-size configuration option to indent-width to express that the formatter uses the option to determine the indentation width AND as tab width.

I first preferred naming the option tab-width but then decided to go with indent-width because:

  • It aligns with the indent-style option
  • It would allow us to write a lint rule that asserts that each indentation uses indent-width spaces.

Closes #7643

Test Plan

Added integration test

This was referenced Oct 20, 2023
Merged
Merged
Merged
Merged
Closed
Merged
Closed
@MichaReiser
Copy link
Member Author

MichaReiser commented Oct 20, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser mentioned this pull request Oct 20, 2023
Merged
charliermarsh
charliermarsh reviewed Oct 20, 2023
View reviewed changes
crates/ruff_workspace/src/options.rs Outdated
/// This option changes the number of spaces the formatter inserts when
/// using soft-tabs (`indent-style = space`).
///
/// PEP8 recommends using 4 spaces per [indentation level](https://peps.python.org/pep-0008/#indentation).
Copy link
Member

@charliermarsh charliermarsh Oct 20, 2023

Choose a reason for hiding this comment

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

Nit: We typically stylize as PEP 8.

Copy link
Member Author

@MichaReiser MichaReiser Oct 20, 2023

Choose a reason for hiding this comment

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

I also fixed the reference to PEP257

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

dhruvmanila
dhruvmanila approved these changes Oct 20, 2023
View reviewed changes
@dhruvmanila dhruvmanila added the configuration Related to settings and configuration label Oct 20, 2023
@MichaReiser MichaReiser mentioned this pull request Oct 20, 2023
Merged
@MichaReiser MichaReiser force-pushed the rename-length-options branch from 596a8ad to bcfb73b Compare October 20, 2023 05:09
@MichaReiser MichaReiser force-pushed the rename-tab-size-to-tab-width branch from e7d0144 to 213ea75 Compare October 20, 2023 05:13
@MichaReiser MichaReiser changed the base branch from rename-length-options to add-formatter-to-line-length-doc October 24, 2023 03:20
@MichaReiser MichaReiser force-pushed the rename-tab-size-to-tab-width branch from 213ea75 to c567862 Compare October 24, 2023 03:20
@MichaReiser MichaReiser mentioned this pull request Oct 24, 2023
Merged
@MichaReiser MichaReiser force-pushed the rename-tab-size-to-tab-width branch from c567862 to 53fcb16 Compare October 24, 2023 03:26
MichaReiser
MichaReiser commented Oct 24, 2023
View reviewed changes
crates/ruff_linter/src/fix/edits.rs
locator: &Locator,
line_length: LineLength,
tab_size: TabSize,
tab_size: IndentWidth,
Copy link
Member Author

@MichaReiser MichaReiser Oct 24, 2023

Choose a reason for hiding this comment

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

We could rename all these fields but here the indent-width is used as tab-width only, so keeping it as is felt correct.

@MichaReiser MichaReiser force-pushed the add-formatter-to-line-length-doc branch from 1c1aed4 to de85ade Compare October 24, 2023 07:45
@MichaReiser MichaReiser force-pushed the rename-tab-size-to-tab-width branch from 53fcb16 to 0df0d26 Compare October 24, 2023 07:47
Base automatically changed from add-formatter-to-line-length-doc to main October 24, 2023 07:55
@MichaReiser
Copy link
Member Author

MichaReiser commented Oct 24, 2023 •
edited
Loading

Merge activity

  • Oct 24, 4:06 AM: Graphite rebased this pull request as part of a merge.
  • Oct 24, 4:14 AM: Graphite couldn't merge this PR because it had conflicts with the trunk branch.

@MichaReiser MichaReiser force-pushed the rename-tab-size-to-tab-width branch from 0df0d26 to a205ebf Compare October 24, 2023 08:06
@MichaReiser MichaReiser force-pushed the rename-tab-size-to-tab-width branch from a205ebf to b2649f2 Compare October 24, 2023 08:28
@MichaReiser
Copy link
Member Author

MichaReiser commented Oct 24, 2023

@charliermarsh feel free to hit merge when the changes look reasonable to you

@charliermarsh charliermarsh merged commit 84979f9 into main Oct 24, 2023
@charliermarsh charliermarsh deleted the rename-tab-size-to-tab-width branch October 24, 2023 14:01
@zanieb zanieb mentioned this pull request May 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhruvmanila dhruvmanila dhruvmanila approved these changes

@charliermarsh charliermarsh Awaiting requested review from charliermarsh

Assignees

No one assigned

Labels

configuration Related to settings and configuration

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rename tab-size to tab-width

4 participants

@MichaReiser @charliermarsh @dhruvmanila