Skip to content
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.

Sign up for GitHub

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

Fix link missing range issue #82

Closed
wants to merge 1 commit into from
Closed

Fix link missing range issue #82

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/Markdown/Inline Nodes/Inline Containers/Link.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@Kyle-Ye Kyle-Ye Nov 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@franklinsch franklinsch Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

}
}
}
Expand Down