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.

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

Allow anchor <doc:#heading> within same article #652

Merged
merged 1 commit into from
Jul 6, 2023

Conversation

natikgadzhi
Copy link
Contributor

Bug/issue #, if applicable: Closes #632

Summary

This pull request makes it so that articles can contain links to specific headings on the same page, using both <doc:Heading>, and <doc:#Heading> syntax.

Dependencies

None, once the PR is ready, itself — I think there are three failing test cases still.

Testing

I've added a test in testArticleSelfAnchorLinks.swift, and verified this behavior manually, too. To test that a #heading link works, you can add a subheading link to an existing document bundle, and rebuild the docs.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • pending: Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary — N/A for this PR I think?

@natikgadzhi
Copy link
Contributor Author

Tests are blowing up:

❯ ./bin/test
Building for debugging...
[3/3] Compiling plugin Swift-DocC Preview
Build complete! (11.59s)
[1464/1464] Testing SwiftDocCTests.SynchronizationTests/testBlockingWithLock
Test Suite 'Selected tests' started at 2023-07-02 21:28:53.802Test Suite 'SwiftDocCPackageTests.xctest' started at 2023-07-02 21:28:53.804Test Suite 'DocumentationMarkupTests' started at 2023-07-02 21:28:53.804Test Case '-[SwiftDocCTests.DocumentationMarkupTests testComments]' started./Users/nategadzhi/src/apple/swift-docc/Tests/SwiftDocCTests/Model/DocumentationMarkupTests.swift:689: error: -[SwiftDocCTests.DocumentationMarkupTests testComments] : XCTAssertEqual failed: ("Document
├─ Heading level: 1
│  └─ Text "Title"
├─ HTMLBlock
│  <!--Line a-->
├─ Paragraph
│  └─ Text "Line b"
├─ BlockDirective name: "Comment"
│  └─ Paragraph
│     └─ Text "Line c This is a single-line comment"
├─ Paragraph
│  └─ Text "Line d"
├─ BlockDirective name: "Comment"
│  └─ Paragraph
│     └─ Text "Line e"
└─ Paragraph
   └─ Text "Line f"") is not equal to ("Document
├─ Heading level: 1
│  └─ Text "Title"
├─ HTMLBlock
│  <!--Line a-->
├─ Paragraph
│  └─ Text "Line b"
├─ BlockDirective name: "Comment"
│  └─ Paragraph
│     └─ Text "@Comment { Line c This is a sin           mment"
├─ Paragraph
│  └─ Text "Line d"
├─ BlockDirective name: "Comment"
│  └─ Paragraph
│     └─ Text "Line e"
└─ Paragraph
   └─ Text "Line f"")Test Case '-[SwiftDocCTests.DocumentationMarkupTests testComments]' failed (0.020 seconds).Test Suite 'DocumentationMarkupTests' failed at 2023-07-02 21:28:53.823.
	 Executed 1 test, with 1 failure (0 unexpected) in 0.020 (0.020) secondsTest Suite 'SwiftDocCPackageTests.xctest' failed at 2023-07-02 21:28:53.823.
	 Executed 1 test, with 1 failure (0 unexpected) in 0.020 (0.020) secondsTest Suite 'Selected tests' failed at 2023-07-02 21:28:53.823.
	 Executed 1 test, with 1 failure (0 unexpected) in 0.020 (0.021) seconds
Test Suite 'Selected tests' started at 2023-07-02 21:29:12.212Test Suite 'SwiftDocCPackageTests.xctest' started at 2023-07-02 21:29:12.213Test Suite 'WebKitCommunicationBridgeTests' started at 2023-07-02 21:29:12.213Test Case '-[SwiftDocCTests.WebKitCommunicationBridgeTests testMessagesCanBeSent]' started./Users/nategadzhi/src/apple/swift-docc/Tests/SwiftDocCTests/Infrastructure/Communication/WebKitCommunicationBridgeTests.swift:52: error: -[SwiftDocCTests.WebKitCommunicationBridgeTests testMessagesCanBeSent] : XCTAssertEqual failed: ("window.bridge.receive({"data":{"numberLiteral":{"alpha":0.6,"blue":18,"red":16,"green":17},"commentURL":{"red":34,"blue":36,"alpha":0.93,"green":35},"lineHighlight":null,"buildConfigId":{"red":40,"blue":42,"green":41,"alpha":0.95},"buildConfigKeyword":{"blue":39,"green":38,"red":37,"alpha":0.94},"parameterName":{"alpha":0.5,"blue":15,"red":13,"green":14},"docCommentField":{"red":28,"blue":30,"alpha":0.91,"green":29},"stringLiteral":{"blue":21,"red":19,"alpha":0.7,"green":20},"background":{"green":2,"alpha":0.1,"blue":3,"red":1},"comment":{"alpha":0.92,"blue":33,"green":32,"red":31},"keyword":{"blue":9,"alpha":0.3,"green":8,"red":7},"docComment":{"alpha":0.9,"blue":27,"green":26,"red":25},"typeAnnotation":{"alpha":0.8,"red":22,"green":23,"blue":24},"text":{"red":4,"green":5,"blue":6,"alpha":0.2},"identifier":{"red":10,"green":11,"alpha":0.4,"blue":12}},"identifier":"12E883C3-E26C-4771-859E-E0577C7AF98A","type":"codeColors"})") is not equal to ("window.bridge.receive({"data":{"docComment":{"red":25,"alpha":0.9,"green":26,"blue":27},"text":{"red":4,"green":5,"blue":6,"alpha":0.2},"stringLiteral":{"blue":21,"alpha":0.7,"red":19,"green":20},"commentURL":{"alpha":0.93,"green":35,"red":34,"blue":36},"identifier":{"red":10,"alpha":0.4,"green":11,"blue":12},"background":{"green":2,"alpha":0.1,"blue":3,"red":1},"comment":{"alpha":0.92,"red":31,"blue":33,"green":32},"parameterName":{"red":13,"alpha":0.5,"blue":15,"green":14},"buildConfigKeyword":{"blue":39,"alpha":0.94,"green":38,"red":37},"buildConfigId":{"alpha":0.95,"blue":42,"red":40,"green":41},"docCommentField":{"green":29,"blue":30,"alpha":0.91,"red":28},"keyword":{"red":7,"blue":9,"alpha":0.3,"green":8},"lineHighlight":null,"typeAnnotation":{"red":22,"blue":24,"green":23,"alpha":0.8},"numberLiteral":{"red":16,"blue":18,"green":17,"alpha":0.6}},"identifier":"12E883C3-E26C-4771-859E-E0577C7AF98A","type":"codeColors"})")Test Case '-[SwiftDocCTests.WebKitCommunicationBridgeTests testMessagesCanBeSent]' failed (0.024 seconds).Test Suite 'WebKitCommunicationBridgeTests' failed at 2023-07-02 21:29:12.237.
	 Executed 1 test, with 1 failure (0 unexpected) in 0.024 (0.024) secondsTest Suite 'SwiftDocCPackageTests.xctest' failed at 2023-07-02 21:29:12.237.
	 Executed 1 test, with 1 failure (0 unexpected) in 0.024 (0.024) secondsTest Suite 'Selected tests' failed at 2023-07-02 21:29:12.237.
	 Executed 1 test, with 1 failure (0 unexpected) in 0.024 (0.025) seconds

The change I've made makes it so some nodes suddenly won't be added (empty nodes for links paths that include anchor fragment, but nothing before "#"). So if any tests relied on that, they would fail. I'm cautious that this PR may change how the path hierarchy is built, so they potentially may break links in documentation.

I'll look into the tests and see if I can clean them up easily.

@natikgadzhi
Copy link
Contributor Author

The two errors seem completely unrelated to the PR at all. Let's smoke test maybe? Could it be Xcode beta?

@natikgadzhi
Copy link
Contributor Author

Uh-huh. I've also just pushed up #653, and that's a documentation-only change, and I see the same two test failures locally. So I guess it's safe to assume those are not related to this PR ;)

@Kyle-Ye Kyle-Ye changed the title Allow anchor <doc:#heading> within same article, fixes #632 Allow anchor <doc:#heading> within same article Jul 5, 2023
@ethan-kusters
Copy link
Contributor

@swift-ci please test

@d-ronnqvist d-ronnqvist self-requested a review July 5, 2023 18:05
Copy link
Contributor

@d-ronnqvist d-ronnqvist left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this. This looks good to me.

Since the failing tests aren't happening on the CI, I think they are specific to either beta Xcode or a beta OS. Something we should fix, but not related to this PR.

@natikgadzhi
Copy link
Contributor Author

@d-ronnqvist, thank you for the review!

Since the failing tests aren't happening on the CI, I think they are specific to either beta Xcode or a beta OS. Something we should fix, but not related to this PR.

Happy to look into those! But first, I started a bit of exploration in #515 (comment) earlier — would you be willing to review and guide me a bit there if I continue? I'm just learning, and would love to work on areas that are useful while I get the grasp of how things work internally.

@d-ronnqvist
Copy link
Contributor

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit 06431a8 into swiftlang:main Jul 6, 2023
if let hashIndex = components.last?.firstIndex(of: "#") {
let last = components.removeLast()
components.append(last[..<hashIndex])
// Allow anrhor-only links where there's nothing before #.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooops, typo. I'll clean this up when I do another pr /shrug

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.

Links to same-page section that start with "#" fail.
3 participants