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

Allow empty values in the spec #47

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

cxw42
Copy link
Member

@cxw42 cxw42 commented Feb 11, 2024

Note: This PR builds on #46. Please review only the head commit. All other commits are part of #46.

Part of editorconfig/editorconfig#496 .

Update the spec to expressly permit the form

key =

as defining key with an empty string as its value. This is different from not defining key at all.

@ppalaga has written draft tests at editorconfig/editorconfig-core-test#52 .

@cxw42 cxw42 added the enhancement New feature or request label Feb 11, 2024
@cxw42 cxw42 requested a review from ppalaga February 11, 2024 20:53
Copy link

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @cxw42!

@cxw42
Copy link
Member Author

cxw42 commented Sep 15, 2024

Updated: rebased on the modified #46.

index.rst Outdated Show resolved Hide resolved
Copy link

@Vampire Vampire left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@xuhdev xuhdev left a comment

Choose a reason for hiding this comment

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

LGTM other than the comments

index.rst Outdated Show resolved Hide resolved
index.rst Outdated Show resolved Hide resolved
@Vampire
Copy link

Vampire commented Sep 25, 2024

As #46 is through, please rebase continue here. :-)

index.rst Show resolved Hide resolved
@cxw42
Copy link
Member Author

cxw42 commented Sep 28, 2024

@xuhdev @ppalaga I am proceeding with merge based on the approvals above! @xuhdev I have confirmed that the latest reflects your comments.

@cxw42 cxw42 merged commit 6b0263e into editorconfig:master Sep 28, 2024
1 check passed
@cxw42 cxw42 deleted the issue496-allow-empty-values branch September 28, 2024 15:24
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.

4 participants