Supports back references #5
Conversation
… feature-back-refs # Conflicts: # SyntaxKit/Pattern.swift
SyntaxKit/Pattern.swift
Outdated
| // MARK: - Initializers | ||
|
|
||
| init(reference: String, in repository: Repository? = nil, parent: Pattern?, manager: BundleManager) { | ||
| init(reference: String, in repository: Repository? = nil, parent: Pattern?, manager: BundleManager /* not used */) { |
There was a problem hiding this comment.
manager can be removed if it is not used.
I guess this is because the language references are resolved eagerly in Language.
There was a problem hiding this comment.
Its parent Pattern uses this parameter.
SyntaxKit/Pattern.swift
Outdated
| type = .resolved | ||
| } | ||
|
|
||
| func resolveExternalReference(from thisLanguage: Language, in languages: [String: Language], baseName: String?) { | ||
| func resolveExternalReference(from thisLanguage: Language /* not used */, in languages: [String: Language], baseName: String?) { |
There was a problem hiding this comment.
This unused argument can also be removed.
SyntaxKit/RegularExpression.swift
Outdated
| // SyntaxKit | ||
| // | ||
| // Created by Rachel on 2021/2/19. | ||
| // Copyright © 2021 Sam Soffes. All rights reserved. |
There was a problem hiding this comment.
The copyright should be changed here.
SyntaxKit/String+BackReference.swift
Outdated
| } | ||
|
|
||
| // Expand a back-referenced regex string with original content and matches | ||
| func removingBackReferencePlaceholders(content: String, matches: NSTextCheckingResult) -> String { |
There was a problem hiding this comment.
Can you please use character literals here. Hexadecimal is not super readable. Also there might be some convenience function like isDigit or something like that.
| XCTAssertFalse("title: Hello World\ncomments: 24\nposts: \"12\"zz\n".hasBackReferencePlaceholder) | ||
| XCTAssert("title: Hello World\ncomments: 24\nposts: \"12\\3\"zz\n".hasBackReferencePlaceholder) | ||
|
|
||
| XCTAssertEqual("title: Hello World\ncomments: \\24\nposts: \"12\\3\"zz\n".convertToICUBackReferencedRegex(), "title: Hello World\ncomments: $24\nposts: \"12$3\"zz\n") |
There was a problem hiding this comment.
Here you could just write the literals once and check that converting it back produces the original string.
SyntaxKit/RegularExpression.swift
Outdated
| return _expression?.firstMatch(in: string, options: NSRegularExpression.MatchingOptions(rawValue: options.rawValue), range: range) | ||
| } | ||
|
|
||
| func rangeOfFirstMatch(in string: String, options: RegularExpression.MatchingOptions = [], range: NSRange) -> NSRange { |
There was a problem hiding this comment.
This would be clearer if written as
// guard expression
return _expression.rangeOfFirstMatch(in: string, options: NSRegularExpression.MatchingOptions(rawValue: options.rawValue), range: range)
(since the rangeOfFirstMatch will never return nil). That way we know that the interface is not changed.
There was a problem hiding this comment.
In general we want to keep the exact same interface as NSRegularExpression.
| @@ -278,9 +281,9 @@ open class Parser { | |||
| if range.location == NSNotFound { | |||
| continue | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Trailing whitespace needs removal
SyntaxKit/RegularExpression.swift
Outdated
| self._options = options | ||
| self._isTemplate = pattern.hasBackReferencePlaceholder | ||
| if !self._isTemplate { | ||
| self._expression = try NSRegularExpression(pattern: pattern, options: NSRegularExpression.Options(rawValue: options.rawValue)) |
There was a problem hiding this comment.
It would be better to add expandBackReferences to this class and not expose the expression at all. It would also be good to have the methods NSRegularExpression methods below fail if expression is nil.
SyntaxKit/Parser.swift
Outdated
| /// - parameter begin: The match result of the beginning | ||
| /// | ||
| /// - returns: The regular expression string with its back reference placeholders expanded. | ||
| private func expandBackReferences(from regex: RegularExpression, beginResults begin: ResultSet?) -> RegularExpression { |
There was a problem hiding this comment.
This should be removed as commented before.
|
Close this in order to open another |
Fix some caveats mentioned here: soffes#18