Skip to content

Respect tab-size setting in formatter#8006

Merged
MichaReiser merged 1 commit intomainfrom
formatter-respect-tab-size
Oct 18, 2023
Merged

Respect tab-size setting in formatter#8006
MichaReiser merged 1 commit intomainfrom
formatter-respect-tab-size

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Oct 17, 2023

Summary

This PR changes the formatter to use the global tab-size to configure the number of spaces used for soft-tabs or width of a hard-tab character.

Closes #7648

Open Questions

I'm still somewhat undecided whether we should support changing the number of spaces for soft tabs. I only found few projects that set tab-size = 2 or pyink-indentation = 2.
But we may as well support it for as long as we default to 4 spaces.

Test Plan

Changed integration test

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

MichaReiser commented Oct 17, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser requested review from charliermarsh, konstin and zanieb and removed request for konstin October 17, 2023 10:17
@MichaReiser MichaReiser added configuration Related to settings and configuration formatter Related to the formatter labels Oct 17, 2023
@MichaReiser MichaReiser force-pushed the formatter-respect-tab-size branch from 108ddc7 to 366d2be Compare October 17, 2023 10:24
This was referenced Oct 17, 2023
Closed
Closed
@MichaReiser MichaReiser mentioned this pull request Oct 18, 2023
Merged
Base automatically changed from lint-preview-option to main October 18, 2023 01:26
@MichaReiser MichaReiser changed the base branch from main to lint-preview-option October 18, 2023 01:34
@MichaReiser MichaReiser force-pushed the formatter-respect-tab-size branch from 366d2be to a583805 Compare October 18, 2023 01:34
@MichaReiser MichaReiser changed the base branch from lint-preview-option to main October 18, 2023 03:13
@MichaReiser MichaReiser mentioned this pull request Oct 18, 2023
Merged
@MichaReiser MichaReiser force-pushed the formatter-respect-tab-size branch from a583805 to f1b00ca Compare October 18, 2023 06:30
@github-actions
Copy link
Contributor

github-actions bot commented Oct 18, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@MichaReiser MichaReiser mentioned this pull request Oct 18, 2023
Merged
charliermarsh
charliermarsh approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Cool, so this both respects the tab width for tab-based indentation and allows changing the number of spaces?

@eli-schwartz
Copy link
Contributor

eli-schwartz commented Oct 18, 2023

Maybe this should look at .editorconfig as well?

@MichaReiser
Copy link
Member Author

MichaReiser commented Oct 18, 2023 •
edited
Loading

Cool, so this both respects the tab width for tab-based indentation and allows changing the number of spaces?

That's correct. It's the one setting that configures the indentation.

Maybe this should look at .editorconfig as well?

We may want to support a ruff init command eventually that copies the relevant settings from the editorconfig. We want to avoid reading the editorconfig ourselves because it then becomes unclear how it would interact with configuration inheritance and it slows down the file traversal (because it increases the number of configuration files that ruff needs to probe for every directory)

@MichaReiser MichaReiser merged commit 4786aba into main Oct 18, 2023
@MichaReiser MichaReiser deleted the formatter-respect-tab-size branch October 18, 2023 23:48
@p-linnane p-linnane mentioned this pull request Oct 19, 2023
Merged
This was referenced Oct 20, 2023
Merged
Merged
@NeilGirdhar
Copy link

NeilGirdhar commented Oct 20, 2023

I'm still somewhat undecided whether we should support changing the number of spaces for soft tabs. I only found few projects that set tab-size = 2

Do you mean editor-config's indent size? If so, Google's Python style guide has it set to two. So that's practically every Google project.

@MichaReiser
Copy link
Member Author

MichaReiser commented Oct 20, 2023

I'm still somewhat undecided whether we should support changing the number of spaces for soft tabs. I only found few projects that set tab-size = 2

Do you mean editor-config's indent size? If so, Google's Python style guide has it set to two. So that's practically every Google project.

Yes, that's what I was considering. We decided to support configuring the number of spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

@zanieb zanieb Awaiting requested review from zanieb

Assignees

No one assigned

Labels

configuration Related to settings and configuration formatter Related to the formatter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use tab_width option in formatter

4 participants

@MichaReiser @eli-schwartz @NeilGirdhar @charliermarsh