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

Clarify the case sensitivity of .editorconfig file #70

Merged
merged 1 commit into from
Jan 26, 2025
Merged

Clarify the case sensitivity of .editorconfig file #70

merged 1 commit into from
Jan 26, 2025

Conversation

xuhdev
Copy link
Member

@xuhdev xuhdev commented Jan 24, 2025

This has been generally assumed since we don't specifically mention other capitalizations, and core libraries have also assumed so. However, some communities seem to have used capital letters in some places, and it's best if we can clarify.

If we go the other route, as a standard, the current core libraries would break because they don't work consistently with each other. Additionally, core libraries are cumbersome to implement on case-sensitive filesystems, because they always have to use some regex to search the .editorconfig file in a directory, instead of just pointing to the file named .editorconfig.

Close editorconfig/editorconfig#523


📚 Documentation preview 📚: https://editorconfig-specification--70.org.readthedocs.build/

index.rst Show resolved Hide resolved
index.rst Show resolved Hide resolved
@xuhdev xuhdev force-pushed the case branch 3 times, most recently from ff915a4 to 2af728e Compare January 25, 2025 02:53
@xuhdev xuhdev requested a review from cxw42 January 25, 2025 02:55
xuhdev added a commit to editorconfig/editorconfig.github.com that referenced this pull request Jan 25, 2025
index.rst Outdated

As noted above, the ``.editorconfig`` filename should be lowercased. On some
platforms, opening a file with a different capitalization results in opening
the same file with lowercased file names. On such a platform, a Core may
Copy link
Member

Choose a reason for hiding this comment

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

Looks good generally. My only concern is the use of "may", since that is the status-quo behaviour. Could we strengthen it? Maybe:

"On such a platform, the core must ask the operating system for .editorconfig (all-lowercase), and must process any file the operating system provides in response."

That way somebody couldn't write a core that only accepted .editorConfig and claim conformance with this paragraph.

Copy link
Member Author

Choose a reason for hiding this comment

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

(I'm also thinking that this subsection should also go to vote -- While we can clarify the lowercase required as we thought they were implicit, as with many other spec document, how Cores should behave on a case-insensitive system is a real change. I can separate this to another PR.)

I improved the wording further more due to it's ambiguity.

I think it's OK to not strengthen this behavior, because users are expected to use all lowercase name, and the rationale to allow a Core to have the said behavior is discussed in the informative paragraph: It is meant to save excessive works from Core. If a Core determines that it's cheap to do so -- I don't know if such a platform exists -- we shouldn't stop them to do so (: What do you think?

Some platforms, such as some cross-platform dynamic programming languages, may introduce unusual behavior to increase portability. They may not be mainstream but by allowing some flexibility here can prevent some implementation difficulties.

Additionally, if some users use .editorConfig, we should probably encourage them to correct that instead of fortify because it will break on some systems.

Copy link
Member

Choose a reason for hiding this comment

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

how Cores should behave on a case-insensitive system is a real change

That's a fair point. I accept this PR as a description of the status quo. Thanks!

xuhdev added a commit to editorconfig/editorconfig.github.com that referenced this pull request Jan 25, 2025
This has been generally assumed since we don't specifically mention other
capitalizations, and core libraries have also assumed so. However, some
communities seem to have used capital letters in some places, and it's best if
we can clarify.

If we go the other route, as a standard, the current core libraries would break
because they don't work consistently with each other. Additionally, core
libraries are cumbersome to implement on case-sensitive filesystems, because
they always have to use some regex to search the `.editorconfig` file in a
directory, instead of just pointing to the file named `.editorconfig`.

Close editorconfig/editorconfig#523
Copy link
Member

@florianb florianb left a comment

Choose a reason for hiding this comment

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

Thanks a lot @xuhdev - i think this will serve well - especially the additional part about the capitalization.

Regarding your and @cxw42 's discussion about the strengthening i agree to your position - it's probably better to have some room for interpretation for the current situation.

@xuhdev xuhdev requested a review from cxw42 January 26, 2025 03:41
@xuhdev xuhdev merged commit d0312ab into master Jan 26, 2025
1 check passed
@xuhdev xuhdev deleted the case branch January 26, 2025 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Clarify capitalization of .editorconfig file
3 participants