editorconfig/editorconfig#429 Remove section name, key and value length limits #21
Conversation
goodhoko
commented
May 21, 2020
goodhoko
commented
- Remove the upper limit for section name, key and value length.
- Permit implementations to define their own upper limits.
- Add minimum supported lengths each implementation must support.
|
One thing that confuses me is the spec verioning. In the spec it says the version is tagged in this repository, however there're no tags nor releases in this repo. (There're howeever, in the core-tests repo.) |
Your confusion is understandable! We are in the middle of changing the versioning of the specification. |
index.rst
Outdated
| @@ -131,8 +131,8 @@ special characters for wildcard matching: | |||
| The backslash character (``\\``) can be used to escape a character so it is | |||
| not interpreted as a special character. | |||
|
|
|||
| The maximum length of a section name is 4096 characters. All sections | |||
| exceeding this limit are ignored. | |||
| Cores must accept section names with length up to 1024 characters. | |||
There was a problem hiding this comment.
Request: "up to and including 1024". That reduces the chance of an off-by-one error.
There was a problem hiding this comment.
good catch! Fixed. Thanks. .)
index.rst
Outdated
| of a pair value is 255 characters. Any key or value beyond these limits | ||
| shall be ignored. | ||
|
|
||
| Cores must accept keys and values with lengths up to 1024 and 4096 characters respectively. |
…th limits - Remove the upper limit for section name, key and value length. - Permit implementations to define their own upper limits. - Add minimum supported lengths each implementation must support.
goodhoko
force-pushed
the
feature/remove-length-limits
branch
from
536dfd6
to
ba8493a
Compare
There was a problem hiding this comment.
I think this PR is ready for a vote. Edit However, based on editorconfig/editorconfig-core-test#41 (comment) , I think it might be worth also expressly stating in the specification that the section name does not include the [ and ]. What do you think?
Note: corresponding core-test PR is at editorconfig/editorconfig-core-test#41
|
@cxw42 I agree it's better to clarify. Since clarifying |
|
@xuhdev I think we are ready for a vote on the heart of this proposal, which is changing the required lengths that cores must support. However, if you think the comments on this PR are sufficient, it's OK with me! |
|
@cxw42 I'm a bit confused about what you are referring here. Are we going to vote on clarifying the limit or removing the limit? |
|
@xuhdev voting on whether to merge this PR. Are the approvals here enough that it does not need a full vote? |
|
Vote open at editorconfig/editorconfig-vote#13 |
|
Seems like we would have another followup vote later: does removing limit mean we'll count everything or do some truncation? |
|
I think I got it: we used to do truncation but now:
|
|
LGTM |
|
Merged now. Thanks! |
|
@xuhdev @cxw42 Thanks! Are there any additional release tasks to be done? Like tagging the commit and updating the website. .) |
|
@goodhoko I don't think there's anything more to be done for this update, because the spec is mostly for developers not for end users. The spec page has been updated: https://editorconfig-specification.readthedocs.io/en/latest/ |
|
@xuhdev Alright. I got confused (again) by the versioning section of the spec. @cxw42 It was more than half a year since you wrote:
Has nothing changed since then? .) If not, I guess we can really call this done. .) |
