Skip to content

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

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

mportiz08
Copy link
Contributor

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.

Copy link
Member

@marinaaisa marinaaisa left a comment

Choose a reason for hiding this comment

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

The implementation here looks cleaner than in #841 in my opinion, but both are great.
I tested this one too and it works as expected.

I left a minor suggestion.
Thanks!

@QuietMisdreavus
Copy link
Contributor

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 mportiz08 marked this pull request as ready for review June 3, 2024 16:56
@mportiz08 mportiz08 changed the title Highlight unique components of overloaded declarations [Alternate Approach] Highlight unique components of overloaded declarations Jun 3, 2024

<script>
// This component is an optimization that allows us to emit raw text DOM nodes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RIP 🥲

@mportiz08
Copy link
Contributor Author

@swift-ci test

@mportiz08 mportiz08 changed the title Highlight unique components of overloaded declarations Highlight unique components of overloaded declarations [alternate approach] Jun 3, 2024
@mportiz08 mportiz08 marked this pull request as draft June 3, 2024 22:51
@QuietMisdreavus
Copy link
Contributor

@mportiz08 I've merged swiftlang/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.

@mportiz08
Copy link
Contributor Author

@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
Copy link
Contributor

@mportiz08 FYI I'm working on resolving conflicts.

@mportiz08
Copy link
Contributor Author

Replacing this PR with #875

@mportiz08 mportiz08 closed this Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants