Skip to content
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.

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

Tabs auto-conversion #2319

Closed
kattkieru opened this issue Jan 29, 2023 · 18 comments
Closed

Tabs auto-conversion #2319

kattkieru opened this issue Jan 29, 2023 · 18 comments
Labels
plugin Implementing a known but unsupported plugin

Comments

@kattkieru
Copy link

Thanks for doing the podcast— made me aware of this project!

You mentioned on the podcast you were considering auto-formatting options in the future. Much as I like Black, I’d love it more if I could configure it to use tabs instead of spaces. Yes, I’m that guy. No, I don’t need a link to that episode of Silicon Valley.

Also, yes I’m serious! I’d love a setting in ruff —fix that fixes indents and converts to tabs.

@charliermarsh
Copy link
Member

Oh nice! Thanks for listening :)

I do plan on supporting configurable spacing in the auto-formatter -- so you'll be able to choose between tabs and spaces (with configurable depth, e.g., two-space or four-space or whatever). Separately, we could support that as a fixable lint rule, similar to the flake8-quotes rules we have now, which enforce single- and double-quotes and support automated fixing. I have to prioritize some other stuff above that, but I'd be open to reviewing a contribution to that effect if someone ends up looking into it.

(We could frame it as a reimplementation of flake8-tabs.)

@LeonarddeR
Copy link

While I know the reason not to use tabs and personally I'm not that much opinionated, I know that many blind and visually impaired people programmers prefer tabs for indentation. The reason behind this is pretty easy to explain. Blind people use screen reading software to read the contents of the screen, accompanied by a braille display that shows a single line of text in braille. Most common braille displays have 40 cells, meaning they are able to show 40 characters at once. There are also braille displays that have double that size.
Now usually, a tab takes only one cell on a braille display, whereas four spaces take four cells. In other words, having a block of indentation that has 20 spaces in front of the code means that half of the common braille display is swallowed up by spacing, compared to only five cells when using tabs.
I hope this reason can lend some weight to prioritizing this, so projects for the blind and visually impaired can also start considering switching to RUFF.

@zanieb
Copy link
Member

zanieb commented Aug 15, 2023

Hey @LeonarddeR — it seems unlikely that we'll implement this as a lint rule since we're focusing on the formatter (#1904 / #6203). We're still willing to review a contribution though if it's a priority for you.

@LeonarddeR
Copy link

Thanks for the quick reply.
I assume the best way to have support for tabs is add it to both the formatter and as a lint rule, right?

@zanieb
Copy link
Member

zanieb commented Aug 15, 2023

We're not sure how we're going to handle conflicts/overlap between the formatter and the linter yet so I can't say. However, we expect some people to use the linter without the formatter still so probably yeah :)

@LeonarddeR
Copy link

I had a look into the Ruff code and I think I'm going to take the challenge to implement this. However, it might help to set out some lines first:

  1. Flake8-tabs allows setting indent-style to either tab or space, i.e. it also has specific stuff for people who yet prefer spaces. I guess it is enough to start with support for tabs first?
  2. When indentation is set to tabs, we need to disable some pycodestyle rules, e.g. w191. What is the best way to handle rules that conflict with each other?

@zanieb
Copy link
Member

zanieb commented Aug 17, 2023

Exciting!

  1. I think incremental implementations are reasonable
  2. I would expect a user to need to opt-in to these tab rules and we can document the rules they conflict with and encourage people not to use them both. I don't think we have more machinery than that.

@charliermarsh will know better than me though.

@LeonarddeR
Copy link

I have the base settled out but am still a bit struggling with the rule type. There is physical lines, logical lines and tokens. Looks like most rules doing something with indentation are either physical or logical lines based. Given several rules compare the current line with the previous line, I assume I should go with logical?

@MichaReiser
Copy link
Member

@LeonarddeR, thank you for your interest. My understanding from the original issue is that it is mainly about the ability to configure the preferred indentation in the formatter, and this is something we intend on doing (the in-development formatter already supports this).

If I'm correct, you want to extend Ruff by a new lint rule in addition to the formatter support. Would you mind telling us about your use case? Would using the formatter, once it is available, be an option for you? Understanding your use case helps us decide on if such a rule is necessary and how to implement it best (as a new rule, change an existing rule, ...). Thank you.

@LeonarddeR
Copy link

Thanks for chieming in @MichaReiser

The underlying issue behind my investigations is that the NVDA Screen Reader Project now uses Flake8 with flake8_tabs for linting. However, flake8_tabs is no longer maintained and we are stuck on an old version of Flake8 which has limited support in VS Code. We don't use auto formatting yet, and though I"am very in favour of introducing that, the project organization will have to decide on that matter.

Would using the formatter, once it is available, be an option for you?

That really depends on what it supports and how we could combine usage of tabs with the current linting rules. Flake8_tabs replaces several pycodestyle rules (such as W191) with a rule that supports both tabs and spaces. I guess we can extend the pycodestyle rule by making it respect the prefered indent style. We also rely on flake8_tabs to support hanging indents, as vertical alignment is something that is pretty confusing for the blind people who contribute to the NVDA code base. You can find some info on flake8_tabs pycodestyle replacements here.

Long story short, if the auto formatter is able to present us this code style without remaining linter warnings, we'd be able to switch:

def hello(
		param  # Note two tabs indentation
):
	...

some_list = [
	1,
	2,
	3,
]  # Closing bracket doesn't hang

@konstin
Copy link
Member

konstin commented Aug 25, 2023

Hey @LeonarddeR given that you're already here talking about tabs and NVDA: In tabs vs. spaces discussions one of the main points brought up for tabs over spaces is that they are a major accessibility feature for non-visual programmers, e.g. there's prettier's tabs vs. spaces issue which talks a lot about this (prettier/prettier#7475). However for most blind programmers in that issue indentation doesn't really seem to matter:

Given that you work on NVDA, do you have more insight into this? I unfortunately failed to find any reliable resources on this save from the ones linked above.

@LeonarddeR
Copy link

@konstin It is indeed true that tabs are easier for some people accessibility wise. I tried to explain this in #2319 (comment)

Basically, for speech only users, tabs and space shouldn't be a difference, but when braille comes into play, there's a major advantage of tabs as tabs take usually less space on a braille display.

If you want to imagine this as a sighted person, you have to pretend you can only see 40 characters on your screen at a time. Then remember that one level of indentation already takes four characters out of 40, two levels 8 out of 40, etc.

@zanieb
Copy link
Member

zanieb commented Aug 25, 2023

Not to nitpick, just also curious for more information on how this works as someone that does not have experience.

Does the braille tooling not support configuration for display of indentation specifically? If so, is it still better to use tabs for some reason?

@LeonarddeR
Copy link

No problem @zanieb, I think it's actually pretty beneficial and clarifying to go to the bottom of this.

Does the braille tooling not support configuration for display of indentation specifically?

This depends on what screen reader you're using, but the only screen reader I know that supports this only supports setting the tab width, i.e. it allows you to set a tab width of four spaces and then the tab is presented as four spaces rather than just one.
doing it the other way around, i.e. collapse four spaces into one, is unfortunately not feasible as for several reasons:

  1. how would you distinguish four spaces from one space? I know that you can't distinguish one space from one tab either, but then it is at least a real tab you're working with that corresponds to one character (i.e. one byte). This is also the reason why alignement gets very messy in braille. Also how would you present two or three spaces in this case?
  2. You'd have to configure the indent size on a project by project basis. Many yaml files have two spaces of indentation per level, which gets very nasty if your screen reader guesses the indent size wrong.

I can probably think of more issues if I keep on thinking.

@nickserv
Copy link

nickserv commented Sep 5, 2023

Note that tabs are also better for users with partial vision loss and cognitive disabilities, as the visual presentation of an indention can be made wider to make it easier to see or remember without changing the source code again.

@MichaReiser
Copy link
Member

Long story short, if the auto formatter is able to present us this code style without remaining linter warnings, we'd be able to switch:

You won't need to use any formatting-related lints when using Ruff's formatter (we may even decide to disable them all automatically). However, Ruff's an opinionated formatter and it formats your example as:

def hello(
	param,  # Note two tabs indentation
):
    ...


some_list = [
	1,
	2,
	3,
]  #

We as a team haven't yet decided what the acceptance criteria or new formatting-related lint rules are that are in possible conflict with the formatter.

@MichaReiser MichaReiser added the needs-decision Awaiting a decision from a maintainer label Sep 5, 2023
@LeonarddeR
Copy link

Now the formatter is able to format using tabs and it does that job very well, I guess the initial question in this issue has been addressed and it can be closed.
From the ruff docs:

Ruff supports all rules from the F category, and a subset of the E category, omitting those stylistic rules made obsolete by the use of an autoformatter, like Black.

I guess according to this philosophy, W191 (Indentation contains tabs) can already be considered obsolete as every auto formatter out there has a strong opinion about tabs or spaces.
Therefore the only remaining decision that I think has to be made is: what to do with W291 (indentation contains tabs)? i guess it should either be marked obsolete, disable by default when the formatter is set to tab indentation, or should be changed so that it can report spaces in indentation where tab is the preference.

@charliermarsh
Copy link
Member

Good call -- I will close for now.

Our current plan is to mark those rules as incompatible with the formatter in the docs and, in the future, warn when using incompatible linter settings with the formatter (#7646).

@charliermarsh charliermarsh removed the needs-decision Awaiting a decision from a maintainer label Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin Implementing a known but unsupported plugin
Projects
None yet
Development

No branches or pull requests

7 participants