Fix link missing range issue #82
Conversation
|
@swift-ci please test |
| @@ -58,7 +58,7 @@ public extension Link { | |||
| if let d = newValue, d.isEmpty { | |||
| _data = _data.replacingSelf(.link(destination: nil, parsedRange: nil, _data.raw.markup.copyChildren())) | |||
| } else { | |||
| _data = _data.replacingSelf(.link(destination: newValue, parsedRange: nil, _data.raw.markup.copyChildren())) | |||
| _data = _data.replacingSelf(.link(destination: newValue, parsedRange: _data.range, _data.raw.markup.copyChildren())) | |||
There was a problem hiding this comment.
Hm I worry a bit that Swift-Markdown is setting the parsedRange inconsistently across different markup elements. Can you explain why this change is needed? Is there another way clients could get the original range before changing the link's destination?
There was a problem hiding this comment.
Can you explain why this change is needed?
Reason: See swiftlang/swift-docc#271 (comment)
Is there another way clients could get the original range before changing the link's destination?
Other way IMO is to add a new property in Link so that we can differentiate and doc: in the consume site. Or we could add a new property oldParsedRange to store the oldRange every time we update the parsedRange.
There was a problem hiding this comment.
I see. Adding a property to Link that tracks what kind of link it is (i.e., a <> 'auto-link' or a regular []() link) seems totally reasonable to me. That way we don't need to fiddle with ranges.
I believe this would also help when reformatting Markdown. I'm assuming that since we're not tracking the style of link right now, we're losing fidelity when formatting Markdown (<> getting converted to []() or vice versa).
Bug/issue #, if applicable:
With swiftlang/swift-docc#376
Close swiftlang/swift-docc#271
Summary
When updating destination of link, we preserve the old
parsedRangeinstead of setting to nil.The
parsedRangeis indeed out of date and we should not rely on it. But sometimes we need the originalparsedRangeto do some logic.See discussion on swiftlang/swift-docc#271 (comment)
Dependencies
None
Testing
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded