Add rule for single space after period on comments #4624
Conversation
|
/retest |
|
&retest |
| ) | ||
|
|
||
| func violationRanges(in file: SwiftLintFile) -> [NSRange] { | ||
| // Find all comment tokens in the file and regex search them for violations |
There was a problem hiding this comment.
I'd love to see a rule enforcing periods to finalize comments. 😉 Can we have a period after each sentence to be consistent?
There was a problem hiding this comment.
Uhhhh! I like that!
Should it be a separate rule though?
There was a problem hiding this comment.
Well, I don't think such a rule would be feasible. It's hard to recognize full sentences. A comment can also be a link, a code snippet or a few words which don't result into a sentence.
My comment was rather meant as a joke referring to your comments in the new rule ... 😄
There was a problem hiding this comment.
I'm glad that's the case, I had a mini crisis trying to figure out the rule for that 😅
1. Moved changelog to the top. 2. Extracted "isComment" logic to a SyntaxClassification extension. 3. Added a double space on the changelog for proper rendering. 4. Added violation markers to the triggering examples.
| } | ||
|
|
||
| let range = classifiedRange.range.toSourceKittenByteRange() | ||
| .map { $0.range.toSourceKittenByteRange() } |
There was a problem hiding this comment.
The isComment seems to be missing here.
There was a problem hiding this comment.
That's why I get for doing manual copy&paste.
Thank you so much for the thorough review!
I'm kinda surprised by the lack of false positives with this mistake. I mean, the list of examples is not exhaustive but the rule was also run on all those other repos right?
There was a problem hiding this comment.
Yes, the rule is run on all the repositories listed above. The violations are truncated to 135 while there is a total of 531, though. So I guess we either just don't see the violations in comment_spacing or the regex used in the rule is already precise enough.
If you have an idea for a test case that would exactly trigger in such a case, do not hesitate to add it. 😉
| ) | ||
|
|
||
| func violationRanges(in file: SwiftLintFile) -> [NSRange] { | ||
| // Find all comment tokens in the file and regex search them for violations |
There was a problem hiding this comment.
Well, I don't think such a rule would be feasible. It's hard to recognize full sentences. A comment can also be a link, a code snippet or a few words which don't result into a sentence.
My comment was rather meant as a joke referring to your comments in the new rule ... 😄
| return true | ||
| case .none, .keyword, .identifier, .typeIdentifier, .operatorIdentifier, .dollarIdentifier, .integerLiteral, | ||
| .floatingLiteral, .stringLiteral, .stringInterpolationAnchor, .poundDirectiveKeyword, .buildConfigId, | ||
| .attribute, .objectLiteral, .editorPlaceholder: |
There was a problem hiding this comment.
Why the full set of cases here? To make sure the compiler warns if there are new kinds?
|
So… what happens next? 🙃 |
Because even prose needs a style guide, and according to owl.purdue.edu we should be using a single space after punctuation, I'm introducing
period_spacing.The rule simply checks and removes extra spaces after the first space following a period in
.blockComment,.docBlockComment,.lineComment, and.docLineComment.Per the contributing guidelines :
This rule opt-in because it triggers in a lot of the linked repositories and it fits the definition of opt-in-rules nevertheless the rule does not trigger in this repository right now.