Skip to content

Conversation

@Bartheleway
Copy link
Contributor

This aims to patch #47

@schorfES
Copy link
Owner

schorfES commented Oct 9, 2024

Hello @Bartheleway, thank you for supporting the project by opening this PR! The main issue I see with your changes is that they handle 'unset' values for the validator’s settings themselves, rather than just the values parsed from EditorConfig. This approach would alter the entire API of Lintspaces and require updates to the documentation regarding allowed values. To address this, I’ve opened another PR (#295) that focuses solely on handling invalid or 'unset' values from EditorConfig. You’re welcome to participate in that PR as well!

@Bartheleway
Copy link
Contributor Author

I have looked at your PR and indeed this is another possibility. The only drawback I see in your approach is, we add a layer of complexity by translating editorconfig values to something else before processing.

Thinking of issue #49, it will make it impossible to solve. Whereas accepting "unset" would make it possible.

@schorfES
Copy link
Owner

From my perspective, this isn’t another layer of complexity but rather a layer of abstraction. As I mentioned in my comment on #49, the goal isn’t to create a 1:1 feature copy of EditorConfig. However, introducing 'unset' as a potential setting moves us in that direction. I appreciate that you’re considering other issues while making this change, but I believe we can achieve the same result by introducing undefined (as you also noted in the table). While my PR #295 doesn’t yet support undefined as a value, I see this as part of a more holistic change that could be addressed in another PR.

@Bartheleway Bartheleway deleted the patch-setting-unset branch October 27, 2024 14:09
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.

2 participants