Opened 12 years ago
Closed 11 years ago
#28494 closed defect (bug) (fixed)
Multiple strings in .editorconfig section names should be wrapped in curly brackets
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 4.0 | Priority: | normal |
| Severity: | normal | Version: | 4.0 |
| Component: | Build/Test Tools | Keywords: | has-patch commit |
| Focuses: | Cc: |
Description
via @treyhunner EditorConfig co-creator in #bbPress2614
The following sections are invalid:
- [*.json,*.yml]
- [*.txt,wp-config-sample.php]
They should instead be:
- [{*.json,*.yml}]
- [{*.txt,wp-config-sample.php}]
Square brackets for the section and curly braces to denote the either-or conditions in the shell glob.
Attachments (1)
Change History (9)
#3
follow-up:
↓ 4
@
11 years ago
Yes the curly brace syntax was added in 0.11.0 which was released in March 2013.
The current syntax is simply wrong, so any change is an improvement. The only kind of file [*.json,*.yml] will match is something like "word.json,anotherword.yml".
If you really don't want to use the curly brace notation you can always expand each glob into separate sections like jQuery still does in theirs: https://github.com/jquery/jquery/blob/949dfe52bc228efe2d0cb2d696b822ef2a5723d1/.editorconfig
Keep in mind that the curly brace notation is very common now and jQuery was one of the first projects to adopt the notation which is why they still use the old notation.
So in summary: I vote for the curly brace notation, but manually expanding into separate sections would work just fine as well. I'll just be glad to see the notation fixed to something parseable.
#4
in reply to:
↑ 3
;
follow-up:
↓ 5
@
11 years ago
Replying to treyhunner:
Thanks for the feedback Trey,
The current syntax is simply wrong, so any change is an improvement. The only kind of file [*.json,*.yml] will match is something like "word.json,anotherword.yml".
It wasn't wrong when we initially added .editorconfig to WordPress in February ;)
If we patch it using {curly braces}:
- IDE/Text Editors with an up to date implementation of .editorconfig 0.11.0 spec will match those files.
- IDE/Text Editors without an up to date implementation will not match the files correctly.
Is there a list of common IDE/Text Editors support of the 0.11.0 .editorconfig spec? Has it been widely implemented?
If you really don't want to use the curly brace notation you can always expand each glob into separate sections like jQuery still does in theirs: https://github.com/jquery/jquery/blob/949dfe52bc228efe2d0cb2d696b822ef2a5723d1/.editorconfig
Keep in mind that the curly brace notation is very common now and jQuery was one of the first projects to adopt the notation which is why they still use the old notation.
This is an option.
So in summary: I vote for the curly brace notation, but manually expanding into separate sections would work just fine as well. I'll just be glad to see the notation fixed to something parseable.
I'm also in favour of this now, per the files affected listed below this is a tiny set of files affected by this change.
Note: Files affected by this change:
wp-config-sample.phpwp-cli.yml.travis.ymlpackage.json- 17 only 3rd party library
license.txt,COPYING.txt,README.txtand similar named .txt files
#5
in reply to:
↑ 4
;
follow-ups:
↓ 6
↓ 7
@
11 years ago
Replying to netweb:
Replying to treyhunner:
Thanks for the feedback Trey,
The current syntax is simply wrong, so any change is an improvement. The only kind of file [*.json,*.yml] will match is something like "word.json,anotherword.yml".
It wasn't wrong when we initially added .editorconfig to WordPress in February ;)
Actually it was. Commas were not meant to denote separate files until we decided to add brace expansion in March. If those globs ever matched something without a ".json," in the middle of the filename and a ".yml" at the end, it was a bug in the parser being used. I'm fairly sure none of the parsers in February 2013 used commas to denote separate file globs.
If we patch it using {curly braces}:
- IDE/Text Editors with an up to date implementation of .editorconfig 0.11.0 spec will match those files.
- IDE/Text Editors without an up to date implementation will not match the files correctly.
Is there a list of common IDE/Text Editors support of the 0.11.0 .editorconfig spec? Has it been widely implemented?
This is specific to each editor plugin. I believe all of the plugins I know have been updated to 0.11 for some time (almost all since March 2013).
#6
in reply to:
↑ 5
@
11 years ago
Replying to treyhunner:
Replying to netweb:
Replying to treyhunner:
Thanks for the feedback Trey,
The current syntax is simply wrong, so any change is an improvement. The only kind of file [*.json,*.yml] will match is something like "word.json,anotherword.yml".
It wasn't wrong when we initially added .editorconfig to WordPress in February ;)
Actually it was. Commas were not meant to denote separate files until we decided to add brace expansion in March. If those globs ever matched something without a ".json," in the middle of the filename and a ".yml" at the end, it was a bug in the parser being used. I'm fairly sure none of the parsers in February 2013 used commas to denote separate file globs.
Thanks, that pretty much decides things then, it was/is broken for everyone for the files and file types the 28494.patch applies to, so we aren't going to break any back compat with this patch.
If we patch it using {curly braces}:
- IDE/Text Editors with an up to date implementation of .editorconfig 0.11.0 spec will match those files.
- IDE/Text Editors without an up to date implementation will not match the files correctly.
Is there a list of common IDE/Text Editors support of the 0.11.0 .editorconfig spec? Has it been widely implemented?
This is specific to each editor plugin. I believe all of the plugins I know have been updated to 0.11 for some time (almost all since March 2013).
Awesome, thanks.
#7
in reply to:
↑ 5
@
11 years ago
- Keywords commit added
- Milestone changed from Awaiting Review to 4.0
Replying to treyhunner:
It wasn't wrong when we initially added .editorconfig to WordPress in February ;)
Actually it was. Commas were not meant to denote separate files until we decided to add brace expansion in March. If those globs ever matched something without a ".json," in the middle of the filename and a ".yml" at the end, it was a bug in the parser being used. I'm fairly sure none of the parsers in February 2013 used commas to denote separate file globs.
Looks like this was broken in [28695].
The curly syntax was added in 0.11.0 which came out in March. We originally added the .editorconfig in February, so I worry that this change will break editorconfig for developers. We should figure out what happens with various editors using editorconfig with the older version of editorconfigcore before making this change.