-
Notifications
You must be signed in to change notification settings - Fork 247
Ignore sentence terminators inside quotes when applying the 'BeginDocumentationCommentWithOneLineSummary' option. #687
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
Conversation
…umentationCommentWithOneLineSummary' option
if $0.element == "OpenQuote" { | ||
isInsideQuotes = true | ||
} else if $0.element == "CloseQuote" { | ||
isInsideQuotes = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to point out that this won't handle nested quotes correctly, but it looks like the Foundation API itself doesn't categorize them correctly either:
func tags(_ text: String) {
var tokenRanges = [Range<String.Index>]()
let tags = text.linguisticTags(in: text.startIndex..<text.endIndex, scheme: NSLinguisticTagScheme.lexicalClass.rawValue, tokenRanges: &tokenRanges)
print(Array(tags))
}
tags("\"Hello 'world'\"")
// ["OpenQuote", "Interjection", "Whitespace", "OpenQuote", "Noun", "CloseQuote", "OpenQuote"]
So an attempt to increment/decrement a nested quote count won't work as intended. In other words, I wanted to point it out but you don't need to change anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is entirely dependent on the behavior of the Foundation API.
And since linguisticTags
is a deprecated API, it's unlikely that we'll see any improvements to it 🤔
The replacement, NaturalLanguage
, works the same as linguisticTags
, but I've migrated to it in case it's improved in the future.
/// A set of `Diagnostic` that can answer the question 'was there an error?' in O(1). | ||
struct TestSingleQuotes {} | ||
|
||
/// A set of `Diagnostic` that can answer the question “was there an error?” in O(1). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both this example and the one below use smart quotes. Can you add one with regular ASCII (straight) double quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll add it!
@@ -11,6 +11,7 @@ | |||
//===----------------------------------------------------------------------===// | |||
|
|||
import Foundation | |||
import NaturalLanguage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add #if os(macOS)
around this import to match where it's used, since it won't exist on Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I missed it.
9fac08c
to
c0a3702
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! There are probably still some tweaks we could do here, but this is still an improvement over the original cade.
Thank you 😊 |
Resolve #669
When
SentenceTerminator
is present betweenOpenQuote
andCloseQuote
, changed the filtering logic to ignore it and prevent it from being recognized as a new sentence.