specification

specification copied to clipboard

It looks like the current wording still leads to too much confusion: https://github.com/editorconfig/specification/pull/29#discussion_r783382976

I therefore propose to remove the explaining addition about inline comments at all, in favor of a description of the former behavior of EditorConfig parsers.

Dear @xuhdev, it seems like the issue is just not solved yet so i would say we remove the inline comments entirely for clarification. Is this something we should propose a dedicated vote for (i don't think so)?

I don't know if this requires a version bump. It looks like but in my opinion the change "just" makes undefined behavior to defined behavior. And elements like escaping a) weren't defined and b) were handled differently by the implemented INI-parsers.

Sorry for jumping in late! Based on the current tests, it looks like the situation is a bit of a mess :( . I think the current text should stay (i.e., I recommend not merging this PR). Per https://github.com/editorconfig/editorconfig-vote/issues/6, values that look like inline comments are expressly undefined. I think it would be better for people to know it's undefined than to incorrectly assume that, because whole-line comments work, inline comments also work.

No worries @cxw42 - i wish i would see a way to act immediately (and see how that time could be used in a productive way to bring EC forward).

The whole attempt is pinning down a parsing issue in the dotnet roslyn compiler: https://github.com/dotnet/roslyn/issues/44596#issuecomment-1132285547

And while i agreed to the voting in 2019 i changed my mind in general. I think we should move forward and make the init file format and EditorConfig explicit. This said i'd love to see a spec going forward creating the demand for all implementers to align to the spec eventually. This wouldn't require immediate action but for everyone having to implement EC or fixing bugs it would be possible to do it in a explicit and defined way.

I also tend to agree now we better explicit state that inline comments are not supported. Often people simply assume their implementation's choice of the "undefined" behavior. This issue may entrench as time goes by (maybe already too late).

I agree with explicitly removing support for inline comments. Would that require a vote? As far as I can tell, editorconfig/editorconfig-vote#6 still controls.

Some possible text:

A ; or # anywhere other than at the beginning of a line [or after /^\s*/?] does not start a comment, but is part of the text of that line. E.g.,

[*.txt]
foo = editorconfig ;)

gives variable foo the value editorconfig ;) in *.txt files, not the value editorconfig.

I don't think so since "backslash/semicolon/octothorpe escaping" would still be "undefined" (as its not mentioned at all).

I am fine with that proposal - i am not sure if it satifies the requirements of the Roslyn team. But i feel like it's something where i have to build some knowledge about writing technical specifications in english. :grimacing:

I agree with the proposed text. Actually, not defining escaping would become natural because it's no longer needed.

@xuhdev

I agree

Thanks :) . Do we need a formal vote to modify https://github.com/editorconfig/editorconfig-vote/issues/6 ?

Maybe we can avoid modifying https://github.com/editorconfig/editorconfig-vote/issues/6 by appending the text:

Naturally, backslash/semicolon/octothorpe escaping is undefined.

Revised proposed text:

A ; or # anywhere other than at the beginning of a line does not start a comment, but is part of the text of that line. E.g.,

[*.txt]
foo = editorconfig ;)

gives variable foo the value editorconfig ;) in *.txt files, not the value editorconfig.

This specification does not define any "escaping" mechanism for ; or # characters.

Edit I asked in the roslyn issue if the above would meet their needs.

Awesome @cxw42 - thank you so much!

Sounds good to me!

gives variable foo the value editorconfig ;) in *.txt files, not the value editorconfig.

Are you sure this is true? I thought this remains undefined. The reason is that some implementation has already seen it as comment and we don't have the need to force them to change for now.

@xuhdev I am proposing that we define it now in the specification, since it's currently implementation-defined. The tests used to include semicolons in the value - https://github.com/editorconfig/editorconfig-core-test/blob/70840cfaf6a06766ab61e975b8a1fe3b891ee08e/parser/comments.in#L14-L16 . I think Roslyn's use case shows that we need to standardize, or else lose cross-core compatibility of values that include # or ; .

Please let me know if I misunderstood your question!

Separately, the proposed language got some upvotes and no downvotes in the Roslyn issue.

Sorry I lost the context over time. Yes, your proposed text makes sense to me. Thanks for organizing them into well-written text!

Do you wanna make another PR for the proposed text?

Let's just change the attached source of this PR - should i do this?

Absolutely, I have no problem with that

@florianb fine with me - thanks!

👍 thanks a lot, i am on vacation for two weeks. I'll add it after that. If someone wants to take this over to speed it up - please feel free. ☺️

@florianb @xuhdev I have taken the liberty of updating this PR to:

• Implement the changes discussed above; and
• Bump the version from 0.14.0 to 0.15.0.

I added new commits rather than removing the existing ones, for ease of comparison. The new commits are 645214f and 69ac8ff. Let me know what you think!

If this change is approved, we will need to add back in the core tests we previously removed. That is why I thought it would be good to bump the minor version, as a sign that cores may need to change.

@xuhdev would you please merge? I am not sure whether you want to squash. Thanks!

I have WIP editorconfig-core-test updates at https://github.com/cxw42/editorconfig-core-c/tree/no-inline-comments

@xuhdev Thanks! I'll let roslyn know. I will be opening PRs shortly for the core-test and editorconfig-core-c changes :) .

And @florianb --- thanks again for opening this PR and for your flexibility! :+1:

I have to thank you @cxw42 for pushing this forward during my absence! You are absolutely welcome!

• editorconfig-core-test PR https://github.com/editorconfig/editorconfig-core-test/pull/49
• editorconfig-core-c PR (draft) https://github.com/editorconfig/editorconfig-core-c/pull/85