Skip to content

[SymbolGraph] don't emit navigator name if it's the same as subHeading #36095

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

Merged

Conversation

QuietMisdreavus
Copy link
Contributor

@QuietMisdreavus QuietMisdreavus commented Feb 22, 2021

Fixes rdar://70202693

When rendering a symbol graph, often there are situations where the names object has duplicate information in the navigator and subHeading field. Since the fields of names can cascade from subHeading to navigator when the latter is absent, choosing to omit this field in this situation can decrease the size of the symbol graph with no loss of data.

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/duplicate-navigator-subheading branch from 239e291 to 495732e Compare February 22, 2021 22:38
@QuietMisdreavus
Copy link
Contributor Author

@swift-ci Please smoke test

@QuietMisdreavus QuietMisdreavus force-pushed the QuietMisdreavus/duplicate-navigator-subheading branch from 495732e to db3685a Compare March 1, 2021 17:58
@QuietMisdreavus QuietMisdreavus changed the title [SymbolGraph] don't emit subHeading name if it's the same as navigator [SymbolGraph] don't emit navigator name if it's the same as subHeading Mar 1, 2021
@QuietMisdreavus
Copy link
Contributor Author

After speaking with the person who reported this issue, it turns out i misunderstood the way the format was used; the navigator field is the one that is optional, in favor of subHeading. I've updated the commit to switch which field is omitted, and updated the tests in kind.

@swift-ci Please smoke test

@bitjammer
Copy link
Contributor

@swift-ci Smoke test Windows Platform

Copy link
Contributor

@bitjammer bitjammer left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@QuietMisdreavus
Copy link
Contributor Author

The Windows test failure seems to be SR-12376. I'm going to go ahead and merge since macOS and Linux both passed.

@QuietMisdreavus QuietMisdreavus merged commit 9ef2b21 into main Mar 2, 2021
@QuietMisdreavus QuietMisdreavus deleted the QuietMisdreavus/duplicate-navigator-subheading branch March 2, 2021 19:39
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.

2 participants