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.

Sign up for GitHub

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

Jump to bottom

Spec incorrectly prohibits spaces around section-name square brackets #500

Closed
cxw42 opened this issue Feb 11, 2024 · 2 comments · Fixed by editorconfig/specification#46
Closed

Spec incorrectly prohibits spaces around section-name square brackets #500

cxw42 opened this issue Feb 11, 2024 · 2 comments · Fixed by editorconfig/specification#46
Assignees
@cxw42
Labels
bug

Comments

@cxw42
Copy link
Member

cxw42 commented Feb 11, 2024

The spec says:

Section Header: starts with a [ and ends with a ].

  • May not use any non-whitespace characters outside of the surrounding brackets.

However, since the initial commit of editorconfig-core-test, such spaces have been allowed: https://github.com/editorconfig/editorconfig-core-test/blob/6c8fe6815b12f96f4b357d610ee1cd8da074880a/parser/whitespace.in#L36-L42 .

The spec should be corrected to match the existing behaviour of the tests.
@cxw42 cxw42 added the bug label Feb 11, 2024
@cxw42 cxw42 self-assigned this Feb 11, 2024
cxw42 added a commit to cxw42/editorconfig-specification that referenced this issue Feb 11, 2024
@cxw42
Section headers may be followed by trailing whitespace
90b0a62 
Make the spec conform to existing test `spaces_after_section_name`.

Fixes editorconfig/editorconfig#500
@cxw42 cxw42 mentioned this issue Feb 11, 2024
Merged
@Vampire
Copy link

Vampire commented Jul 25, 2024

Actually, while the PR improves the wording, I'd say spec and tests are in sync already regarding this.
The spec forbids non-whitespace outside the brackets, which implies that whitespace is fine, otherwise the spec should say "nothing is allowed".

So imho regarding this issue the PR is only using better wording, explicitly allowing whitespace instead of forbidding non-whitespace. :-)

@cxw42
Copy link
Member Author

cxw42 commented Sep 15, 2024

@Vampire Good point --- I must have not have been thinking clearly when I opened this issue 😅 . I'm going to close this ticket and then revisit the PR.

@cxw42 cxw42 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cxw42 cxw42

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Clarify whitespace handling cxw42/editorconfig-specification
2 participants
@Vampire @cxw42