-
Notifications
You must be signed in to change notification settings - Fork 158
Only curate external non-symbols in different lang #1269
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
6c5b795
to
812c56a
Compare
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless there's some information that I'm missing, this change in behavior feels like it's incorrect to me.
812c56a
to
2c1a2b6
Compare
2c1a2b6
to
dc0f2e2
Compare
@swift-ci please test |
dc0f2e2
to
eef3d1e
Compare
eef3d1e
to
d9b9206
Compare
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good. I think it would be good to add code comments in a few places to highlight that this behavior is a workaround, rather than the intended behavior.
d9b9206
to
0b61e5f
Compare
I've added a note with the link to #240 in the relevant comments, and also left FIXMEs for us to come back to. |
@swift-ci please test |
PR swiftlang#757 enabled DocC to include manually curated non-symbol nodes in the topics section, irrespective of the node language. This allows curating articles across documentation in different languages, even though the article's language is considered to be "Swift". However, the logic was too permissive, and allowed curating non-symbol nodes within the same bundle but across different languages. This is incorrect, since the target page should be annotated with the `SupportedLanguage` directive to enable curating it in a different language. A bespoke mechanism to curate an internal reference correctly is not necessary. This patch tightens the logic to only allow curating external references in a different language than the source page language, and removes the existing test for the undesired behaviour of including local references to cross-language symbols. The changes in PR swiftlang#757 were not propagated to the navigator, resulting in the references being present in the topics section but not in the sidebar. This patch also introduces the curation logic in the navigator to maintain parity with the topics section. The navigator tests for external render nodes have been modified accordingly to reflect this change. rdar://155522179
0b61e5f
to
97924a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you for working through all the feedback.
@swift-ci please test |
Bug/issue #, if applicable: rdar://155522179
Summary
PR #757 enabled DocC to include manually curated non-symbol nodes in the
topics section, irrespective of the node language. This allows curating
articles across documentation in different languages, even though the
article's language is considered to be "Swift". However, the logic was
too permissive, and allowed curating non-symbol nodes within the same
bundle but across different languages. This is incorrect, since the
target page should be annotated with the
SupportedLanguage
directiveto enable curating it in a different language. A bespoke mechanism to
curate an internal reference correctly is not necessary. This patch
tightens the logic to only allow curating external references in a
different language than the source page language, , and removes the
existing test for the undesired behaviour of including local references
to cross-language symbols.
The changes in PR #757 were not propagated to the navigator, resulting
in the references being present in the topics section but not in the
sidebar. This patch also introduces the curation logic in the navigator
to maintain parity with the topics section. The navigator tests for
external render nodes have been modified accordingly to reflect this
change.
Dependencies
None
Testing
The navigator tests in
ExternalRenderNodeTests
have been updated to reflect the curation behaviour.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded