denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
97.48k stars 5.38k forks source link

deno fmt accessibility #3695

Closed brandonkal closed 2 years ago

brandonkal commented 4 years ago

I really like the idea of "one formatting style". People spend too much time on these matters. I also currently use two spaces for JavaScript files as it is the Prettier default.

But thinking more about the accessibility issues with defaulting to a small indent size, deno should reconsider.

Ref: https://github.com/stylelint/stylelint/issues/4246 https://www.reddit.com/r/javascript/comments/c8drjo/nobody_talks_about_the_real_reason_to_use_tabs/

It is hard to read code when it is scrunched to one side. We have less of a need for a small indent size given the move from callbacks to async/await.

Given the above, I'd like to see deno fmt consider defaulting to use tabs before 1.0. When editing a file with tabs, the user can choose whether an indent should visually be displayed as 2, 4, or even 8 spaces. Tabs would also be more consistent with go fmt which the tool is clearly inspired from. I'm not interested in starting a holy war on this though.

I'd also like to see the ability to set the deno fmt CLI arguments as environment variables so users could use deno fmt if they strongly disagree with the defaults.

kitsonk commented 4 years ago

Since deno fmt is configurable, I say we stick with our defaults.

brandonkal commented 4 years ago

But do consider that two spaces is unreadable for many people (this even varies between codebases). The article changed my mind on that. There is a reason TypeScript emits four spaces. Indentation is important. But using spaces forces the visual preference on everyone. For instance, you are fine seeing indents as two characters. My visually impaired friend needs 4 to be productive. By using the indentation character Tab, both of you can be happy. You can even ignore the key itself and your IDE will handle things for you.

Still, I can understand your fear to rock the boat...

For that, see my second suggestion. Allow us to set a

DENO_FMT_ARGS="--use-tabs --arrow-parens=always"

environment variable. That makes things close-to-seamless for codebases that care to be accessible.

David-Else commented 4 years ago

Deno formatting defaults should stick to what Prettier decides, that is the point of Prettier, so everyone can have the same formatting. With 80 characters print width any more than 2 spaces messes things up. It is all a fine balancing act. 80 chars and 2 spaces indent works great on a 1080p screen so you can get 2 columns.

The only exception to using the Prettier's current defaults is of course... it should be single quotes by default! I assume people touch type, the single quote are so much better. I understand that the Prettier team are considering switching to this in the future, so maybe it would be sense to beat them to it :)

Prettier 2.0 (old) Summary:

Things that appear to have consensus:

4102: Change the default value for singleQuote to true

https://github.com/prettier/prettier/issues/3503#issuecomment-359863688

It does not really matter though, as there is a command line option to use single quotes in deno, but I just thought I would nerd out and mention it here :)

I have never heard of the 2 spaces being an accessibility issue before, so I don't think this is a mainstream opinion. I do applaud people thinking about accessibility issues though, it is important.

brandonkal commented 4 years ago

@David-Else 😆 I very much like single quotes as well! The thing is it doesn't really matter there. Touch type with single quotes and prettier will save it as double. I don't have a strong opinion on tabs vs spaces. But it is more accessible.

If you have to use a very large font, you may set tabs to use one column. If you have a 1080p screen and the code isn't terribly complex, you'll set it to two. If there is a lot of branching or you have a 4K screen, you'll set it to four or even 8. This can all be done without changing the file contents which is a distinct advantage. As far as I can tell, prettier treats tabs as 2 columns when printing.

I also believe it depends on what you are doing. For front-end (where more information is conveyed by JSX brackets etc) I've never had an issue with 2 column indents and 80 columns. Now as I write a Kubernetes client with all of that complexity, increasing the visible indent makes it easier to follow the complex TS types and branching.

Still, it is worth looking at before Deno 1.0.

ry commented 4 years ago

2 space indent and 80 columns is what we're sticking with.

Alhadis commented 4 years ago

JSYK, I once gave myself legitimate eye-strain trying to follow 2-space indented code for a pull-request. I actually gave up, and have been contributing less to open source projects because of the prevalence of this horrible tab-stop width.

Not saying "please change the defaults", I'm only lending credence to what @brandonkal's been saying about accessibility. It's a very real issue.

*drops mic*

2 space indent and 80 columns is what we're sticking with.

Not when I read it. 😁

hdodov commented 3 years ago

Sad to see this issue is closed, because I like what the Linux kernel docs say on this:

Tabs are 8 characters, and thus indentations are also 8 characters. There are heretic movements that try to make indentations 4 (or even 2!) characters deep, and that is akin to trying to define the value of PI to be 3.

Rationale: The whole idea behind indentation is to clearly define where a block of control starts and ends. Especially when you’ve been looking at your screen for 20 straight hours, you’ll find it a lot easier to see how the indentation works if you have large indentations.

Now, some people will claim that having 8-character indentations makes the code move too far to the right, and makes it hard to read on a 80-character terminal screen. The answer to that is that if you need more than 3 levels of indentation, you’re screwed anyway, and should fix your program.

In short, 8-char indents make things easier to read, and have the added benefit of warning you when you’re nesting your functions too deep. Heed that warning.

I used 4 spaces, then 2 (because of Standard JS), then switched to tabs when I joined a team. The reason is that with tabs, everyone is happy because code looks the way they want to and can read it comfortably, but most importantly, nobody has to agree or even care about anyone else's preferred indentation size. It works universally. And working universally sounds like a good feature for an opinionated code formatter to have.

I'm all about consistency and opinionated code style. But I think that code style encompasses syntax preferences like semicolons or no, single or double quotes, etc. Indentation is more about accessibility than code style, and tabs are the obvious winner.

retog commented 3 years ago

Very sad, no reason provided by people without disabilities. Indent with tabs, recommend to display them with the width of two spaces and count them as two characters when checking for the 80 characters line limit. What disadvantages would this rule have? It would make the code significantly more accessible to many.

kitsonk commented 3 years ago

I think we need to reopen this issue. We have talked amongst ourselves that this is important consideration and have discussed allowing deno fmt to potentially support a minimalistic set of options aligned to what prettier supports.

Having this issue closed makes it look like we don't care about these things and aren't talking about them.

retog commented 3 years ago

Thanks, @kitsonk dor reopening. I'm also "all about consistency and opinionated code style", so I would prefer a non-optional choice for tabs, which would allow everyone to read existing code without having to reformat.

Alhadis commented 3 years ago

@retog Part of me disagrees about making anything non-optional. Even though there isn't a single good reason to use spaces instead of tabs for indentation, some people are rooted in this "tabs = evil" mentality from the dark ages (when people resorted to mixed tabs/spaces because modern text-editors/IDEs weren't around to make tab stops easily configurable). These users—or at least those oblivious to the existence of expand(1) and friends—are going to expect an option to use spaces for… fuck knows what reason. 🤷

Then again, if most users aren't complaining about having 2-space tabstops shoved down their throats, I doubt they'll complain or even notice if tabs are used instead (since they'll probably have their editors configured to use 2-column tabstops anyway 😂). Hell, I say go for it.

nayeemrmn commented 3 years ago

there isn't a single good reason to use spaces instead of tabs for indentation

Actually invisibly multi-column characters aren't a great text editing experience, for some people.

Alhadis commented 3 years ago

Actually invisibly multi-column characters aren't a great text editing experience, for some people.

Citation (or at least some elaboration) needed.

It's still also prioritising one's own preferences over user accessibility, and that's a weak argument for a coding style intended to be shared between collaborators.

nayeemrmn commented 3 years ago

Actually invisibly multi-column characters aren't a great text editing experience, for some people.

Citation (or at least some elaboration) needed.

Not sure how "citation" is appropriate in this context but elaboration sure. I was talking about how tab characters are difficult for me when they get displaced with spaces or I need to do multi-cursor edits across multiple lines (that thing when you drag with the middle mouse button in vscode). It's definitely still a preference, but please keep track of the fact that I'm responding to a steer in the discussion towards "a non-optional choice for tabs". I'm for configurability and arguing for that.

Alhadis commented 3 years ago

Apologies for the diatribe. I agree that keeping things configurable, no matter their defaults, is of chief importance. 👍

hdodov commented 3 years ago

Maybe Deno could auto-detect and pick one or the other. In a space-indented file, it's not likely to have a tab character, therefore:

It doesn't sound good to search the entire source file and there could be edge cases (using tab characters in a comment), but I'm throwing it out there.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

skuridin commented 3 years ago

Since deno fmt is configurable, I say we stick with our defaults.

Is it? Cannot find any documentation.

v1.14.0 was just released with the fmt configuration. Looks like this issue can be closed.

ry commented 2 years ago

Happy to support tabs as an option (https://github.com/denoland/deno/pull/13266), but we're not doing it by default.