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

Switch to hard tabs for indentation #1298

Closed
Cherry opened this issue Jun 19, 2022 · 3 comments · Fixed by #1309 or #1336
Closed

Switch to hard tabs for indentation #1298

Cherry opened this issue Jun 19, 2022 · 3 comments · Fixed by #1309 or #1336
Assignees
Labels
discussion This issue is being used to discuss a topic rather than track a feature or bug.

Comments

@Cherry
Copy link
Contributor

Cherry commented Jun 19, 2022

Everyone has their own opinions about how much visual indentation they want in their code. Some prefer 2 spaces, some prefer 4, and then some users may be visually impaired and need higher font sizes with smaller tab widths, etc.

There's been a tonne of debate over the years about which is better, and it's always come to down to arguments that are opinion based, but the one major and consequential difference between the two is this:

Tabs allow individuals in the same codebase to select their indentation widths across every tool.

To affirm the accessibility issue, I'd recommend reading through https://www.reddit.com/r/javascript/comments/c8drjo/nobody_talks_about_the_real_reason_to_use_tabs/:

coworkers who unfortunately are highly visually impaired, and each has a different visual impairment:

  • one of them uses tab-width 1 because he uses such a gigantic font-size
  • the other uses tab-width 8 and a really wide monitor
  • these guys have serious problems using codebases with spaces, they have to convert, do their work, and then unconvert before committing

Other references:

Prettier is likely to switch to useTabs by default with the next major version too, for this exact reason: prettier/prettier#7475

Other than the obvious code change differences, shipping an .editorconfig in this repo for some sensible defaults would probably be a good idea too, to combat one of the most common counters to this, which is how some tools render tabs by default. This way, tools like GitHub can render tabs with a 4 character width by default instead of the usual 8, but folks can still customise this themselves if they so choose. Something like this:

# http://editorconfig.org
root = true

[*]
indent_style = tab
tab_width = 4

I'd like to avoid any debate around personal preference here and focus on the accessibility issue of continuing to indent code with spaces. This discussion should probably be had for most of Cloudflare's JS/TS projects, but since wrangler is one of the biggest, I figured it best to start here.

@threepointone
Copy link
Contributor

No need to have too long a discussion either, I think we should do this (and honestly it sucks that we haven't so far). Just a matter of timing. Now that we've shipped, I'll bring this up with the team and we'll try to do this soon.

@threepointone
Copy link
Contributor

Thanks @Cherry for bringing this up, I appreciate you deeply for filing this issue and making a strong case.

@threepointone threepointone added the discussion This issue is being used to discuss a topic rather than track a feature or bug. label Jun 19, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Jun 20, 2022
@petebacondarwin petebacondarwin self-assigned this Jun 20, 2022
@petebacondarwin
Copy link
Contributor

Created a PR to discuss at our check in meeting today.

petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Jun 20, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Jun 20, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Jun 27, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Jun 28, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue Jun 28, 2022
petebacondarwin added a commit that referenced this issue Jun 28, 2022
* refactor: switch to tab indentation rather than spaces
* chore: update to latest prettier
* style: update files to new tab indentation
* style: convert all source code indentation to tabs

Fixes #1298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is being used to discuss a topic rather than track a feature or bug.
Projects
Archived in project
3 participants