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

introduce .editorconfig file #148

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

maximecolin
Copy link
Member

@maximecolin maximecolin commented Sep 2, 2021

This PR aims to generalize usage of .editorconfig file on Elao projects and to prevent IDE config to non consistent with project practice in terms of indentation.

Add a basic .editorconfig file to the manala.app recipe.

https://editorconfig.org/

These rules will be created by manala init, then project maintainers can modify them, manala will not overwrite them.

The base is 4 spaces for indent all files, tabs for makefile.

Need to be completed by specific rules:

  • How many spaces for javascript files ?
  • How many spaces for sass/css files ?
  • How many spaces for yaml files ?
  • Specifics indent for some files like .manala.yaml ? *.orm.yaml ? translations ?

@maximecolin maximecolin added the enhancement New feature or request label Sep 2, 2021
@maximecolin maximecolin self-assigned this Sep 2, 2021
@maximecolin maximecolin force-pushed the elao.app/editorconfig branch from 5c7ad92 to d06f661 Compare September 2, 2021 14:13
@ogizanagi
Copy link
Member

ogizanagi commented Sep 2, 2021

My own prefs:

How many spaces for javascript files ? => 2
How many spaces for sass/css files ? => 2
How many spaces for yaml files ? => 2
Specifics indent for some files like .manala.yaml ? *.orm.yaml ? translations ? => 4 for Symfony files

Should we add as well:

[*.md]
trim_trailing_whitespace = false # keep end of line double-space for markdown EOL
max_line_length = 120 # wrap after X chars

[*.json]
indent_style = space
indent_size = 4

?

What about max_line_length for JS/PHP ?
On some JS projects, we have eslint enforcing 80 chars max (but I think this is too short for nowodays screens, and very frustrating).

@maximecolin
Copy link
Member Author

Ha! max_line_length, I was never confortable with this rule. It's a bit frustrating when your line is 121 length, can be displayed entirely on your screen but the rule is 120 so you have to break it 😅

For markdown, not sure it's a good idea. For full text markdown, I use soft wrap, no nead for enforced max length, soft wrap will adapt the text to the screen. Soft wrap or max length can be problematic when you have table or links in your markdown, it's very common to have a link or a table row that is longer than 120 chars and they don't wrap nicely (not even sure they can't be cut). For this, I have no solution except not enforcing max length.

For code, on my 13" screen, with a pretty large left panel, I can display 120 chars, 165 without the menu, on a bigger screen I think 200 easy.

After looking my last project code, I (almost) never write more than 120 chars per line so I vote for 120.

@ogizanagi
Copy link
Member

For markdown, not sure it's a good idea. For full text markdown, I use soft wrap, no nead for enforced max length, soft wrap will adapt the text to the screen. Soft wrap or max length can be problematic when you have table or links in your markdown, it's very common to have a link or a table row that is longer than 120 chars and they don't wrap nicely (not even sure they can't be cut). For this, I have no solution except not enforcing max length.

Actually it's quiet a common thing to restrain the max line length for markdown, rst and other docs formats ; Symfony even restrict it to 80 chars, but note this is not a "hard" rule, if the line is 81 chars or contains a link, code block, …, it'll be allowed as-is. Same for tables, it's not really an issue. If it cannot be wrapped, it won't. It mainly enforces paragraphs.
This improves readability vs soft-wrap. But it also have drawbacks since it'll likely generate more diffs when editing the document.
It impact IDE's auto-formatting if you use it (PHPStorm handles it well regarding the issues you mentioned), but especially, it configures the max line length visual indicator as well.

After looking my last project code, I (almost) never write more than 120 chars per line so I vote for 120.

👍🏻

@maximecolin maximecolin force-pushed the elao.app/editorconfig branch 2 times, most recently from 5ec4a32 to ee0081f Compare September 3, 2021 12:20
# Symfony #
###########

[*.twig]
Copy link
Member

@ogizanagi ogizanagi Sep 3, 2021

Choose a reason for hiding this comment

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

Suggested change
[*.twig]
[*.html.twig]

?

Sometimes we might generate something else than HTML, and I think the generated format indent matters more than Twig indent. We use 4 for twig in most cases because we use 4 for HTML, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants