lukeed / ley

(WIP) Driver-agnostic database migrations
MIT License
260 stars 14 forks source link

Add option for space-based indentation in `ley new` files, make default? #16

Closed karlhorky closed 3 years ago

karlhorky commented 3 years ago

Hi @lukeed 👋 Hope you are well and everything is going great.

Would it be possible to have an option for ley new ... commands that allow setting the indentation to space- or tab-based?

Also, since the common JS style is space-based indentation, would you also consider making this the default? If you would agree here, the option name could also be --use-tabs, analogous to the Prettier option

lukeed commented 3 years ago

Hey, thank you, but things have been better to be perfectly honest 😬

You should already be using .editorconfig files & IDE plugins. It will auto-convert all files (by extension match) to your specified settings. So as soon as you write & save your migration contents, any whitespace will be converted to your preferences.

That said, I won't add a CLI flag for this. It's not a ley concern, and there are better tools/solutions out there for this. Also, for the record, common JS style is space-based indentation – this is completely not true. Whitespace has absolutely nothing to do with the module format.. you just may be seeing space-based indentation more often than not.

FWIW, tabs are more accessible and team-friendly. A space is always 1-character width. Tabs are configurable and can have local overrides w/o affecting team members' preferences (see .editorconfig once again).

Sorry, but thank you for the suggestion!

karlhorky commented 3 years ago

You should already be using .editorconfig files & IDE plugins. It will auto-convert all files (by extension match) to your specified settings. So as soon as you write & save your migration contents, any whitespace will be converted to your preferences.

Hm, I just tried this out, and I couldn't get the VS Code EditorConfig extension to do what you're suggesting it can. I used an .editorconfig file with the following content. There was no auto-formatting of the existing content in the file:

[*.js]
indent_style = space
indent_size = 2

I do have Prettier as well, which gets the user part of the way, but it (or maybe VS Code?) has a weird problem with it:

  1. Create a new migration file (this uses tabs)
  2. Save the file (tabs converted to spaces by Prettier 🙌)
  3. Change the // <insert magic here> to await client``, move the cursor inside the backticks, and then hit enter
  4. Tabs are back (inside the template string)! 😫

I think it may have something to do with the auto-detection of the indentation when the file loads.


Also, for the record, common JS style is space-based indentation – this is completely not true. Whitespace has absolutely nothing to do with the module format.. you just may be seeing space-based indentation more often than not.

Ah, I think my words were confusing here. I didn't mean anything to do with the module format. I meant that the most common style in JS code is space indentation.

But no problem, I don't mean to start a religious war about this.


I have an alternative that I'd like to propose: just avoid adding indentation. Here's a PR: https://github.com/lukeed/ley/pull/17

karlhorky commented 3 years ago

Reopened a new PR with a concise description of the problem at #21