-
Notifications
You must be signed in to change notification settings - Fork 228
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
Add whitespaceOnly option to configuration file #195
base: swift-5.2-branch
Are you sure you want to change the base?
Add whitespaceOnly option to configuration file #195
Conversation
Allow configuring whether formatter should add or remove trailing commas in lists.
Thanks for the contribution! I don't think we want to call a user-configurable trailing comma feature something as general as "whitespace only". We do that in the internal implementation because the linter half of the pretty printer works by looking at runs of whitespace but expects the non-whitespace characters to be the same, and the formatted output for the linter is never otherwise used, but we don't have to (and shouldn't) make the user-facing feature work the same way. If we ever add other situations in the pretty printer where it inserts or removes non-whitespace characters (as a hypothetical, we could do something like add parentheses around long wrapped expressions), we don't want a single setting to turn all of those features on/off. Instead, let's call it
This concept is general enough that I can see it applying to future configuration settings, so I think defining a general With that change, you'll need to make the actual change on the pretty-printer side in Oh, and please make sure your PR is set to merge into |
Possibly even more flexible format is needed, something similar to ESLint's:
ESLint's first parameter is whether the rule results in an error or in a warning, other parameters are rule-specific. Some eslint rules (import sort order etc) require relatively complex configuration. |
All lint violations are emitted at the Swift has fewer trailing comma contexts that apply than Javascript/Typescript—only array and dictionary literals. I personally don't see the need to make those two things separately configurable (other folks may disagree), so in the interest of avoiding feature creep, I think it's better to start with just making the feature apply to both arrays and dictionaries in the same way. If we do need a more complex configuration sub-object in the future (for example, if we determined there was a need to format trailing commas for arrays and dictionaries differently), then that's fairly straightforward to do without breaking users: the custom implementation of |
…tlang#195) * Fix public extensions exposing nested code of all access levels * Inline access checking closure Co-authored-by: Max Desiatov <[email protected]> * Update Changelog.md * Update Changelog.md * Add testComputedPropertiesInPublicExtension * Add testComputedPropertiesWithMultipleAccessModifiersInPublicExtension * Do not skip properties which have limited access of setters only * Add missing case and fix incorrect checks in access level tests Co-authored-by: Max Desiatov <[email protected]> Co-authored-by: Mattt <[email protected]>
Allows configuring whether formatter should add or remove trailing commas in lists.