swiftlang / swift-docc-render

Web renderer for Swift-DocC documentation.
Apache License 2.0
312 stars 52 forks source link

Highlight unique components of overloaded declarations [alternate approach] #847

Closed mportiz08 closed 4 months ago

mportiz08 commented 5 months ago

This implements the same rendering of highlighted tokens as #841 but using an alternate Render JSON schema where each token may possibly have a highlightDiff boolean flag instead of introducing a new token kind to represent highlights.

I'm not sure yet which approach the DocC team is going to decide on yet, so I'm opening a draft PR for both options and will close one depending on what they decide on.

Testing

Same instructions as #841 except make sure to use the older commit 33388c3c7987904e0aec58e4035fea14bcd19965 from the swift-docc branch when building DocC instead of the latest commit from the mentioned branch.

QuietMisdreavus commented 5 months ago

Dang, this does look a lot nicer than the highlightDiff span approach in #841. I think i was the last major holdout for that approach, on the assumption that it would make integration easier. 😅 Seeing this, i think i'd prefer this version of the implementation on the Swift-DocC side as well, since the text-splitting and -recombining code was a lot simpler on single tokens! Thanks for writing this!

mportiz08 commented 5 months ago

@swift-ci test

QuietMisdreavus commented 4 months ago

@mportiz08 I've merged https://github.com/apple/swift-docc/pull/928 with the "highlight": "changed" field, so this PR can land whenever you're ready. Looks like there's a merge conflict now.

mportiz08 commented 4 months ago

@mportiz08 I've merged apple/swift-docc#928 with the "highlight": "changed" field, so this PR can land whenever you're ready. Looks like there's a merge conflict now.

Sorry yes. We are anticipating needing to update this PR due to major refactoring of declaration components as part of #832 . I was hoping we could sync this change across both repos at the same time, but we can prioritize updating this PR now to try and avoid issues now that it has been merged in DocC.

hqhhuang commented 4 months ago

@mportiz08 FYI I'm working on resolving conflicts.

mportiz08 commented 4 months ago

Replacing this PR with #875