Custom link titles authored with []() syntax for doc links are silently ignored

[franklinsch]

Custom link titles for doc links are silently ignored.

If I write a doc link with a title like:

Check out [this great function](doc:Sloth/eat(_:quantity:)).

the rendered output is

Check out eat(_:quantity:).

We should use the user-provided title, per the original Markdown spec.

Checklist

• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue.

Steps to Reproduce

Author a doc link to a symbol or article using a custom title, as described above. Build documentation and notice that in the rendered documentation, the custom title isn't used for the link title.

Swift-DocC Version Information

Swift-DocC version: 5.6
Swift Compiler version: N/A

Radar: rdar://79992417

[franklinsch]

Note: this seems related to #177.

[ktoso]

Please also support this in "## Topics" because often a "big topic page" might be linking to "these are the few very specific functions you should look at" and without curation those might look very out of place.

For example:

## Security
...

## Topics

- ``SecuritySettings/tls``

only shows up as:

• tls

today which looks very weird, so I'd like to spell it as SecuritySettings.tls for developers so they know "aha, that's a setting I should look at".

[Kyle-Ye]

I want to discuss some implementation detail and the potential fix I tried here.

The problem is we'll use Link.children's property as the "overridingTitle" and "overridingTitleInlineContent" for normal links whose schema is not doc.

But for "doc" schema links we'll always use nil for the "overridingTitle" and "overridingTitleInlineContent" (See line 133 below)

https://github.com/apple/swift-docc/blob/566a1770f4919fd9392d7980e924ff170dd0b7c3/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift#L85-L133

The simple fix is just using the same logic for doc schema links

let linkTitleInlineContent = link.children.reduce(into: [], { result, child in result.append(contentsOf: visit(child))}) as! [RenderInlineContent]
let plainTextLinkTitle = linkTitleInlineContent.plainText

return [RenderInlineContent.reference(identifier: .init(resolved.absoluteString), isActive: true, overridingTitle: overridingTitle, overridingTitleInlineContent: overridingTitleInlineContent)]

But this will cause some tests to fail and maybe break some original expected behavior.

So we could improve this by adding some checks here: (Set nil if the title is matched)

let linkTitleInlineContent = link.children.reduce(into: [], { result, child in result.append(contentsOf: visit(child))}) as! [RenderInlineContent]
let plainTextLinkTitle = linkTitleInlineContent.plainText

let overridingTitle = plainTextLinkTitle == destination ? nil : plainTextLinkTitle
let overridingTitleInlineContent = // Alike logic

return [RenderInlineContent.reference(identifier: .init(resolved.absoluteString), isActive: true, overridingTitle: overridingTitle, overridingTitleInlineContent: overridingTitleInlineContent)]

However, after doing so, we will still have one test case fail: IndexingTests/testTutorial

And it was because we did some rewrite to link's destination when trying to resolve the link while leaving its other children unchanged.

Original link

├─ Link destination: \"doc:/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"
│  └─ Text \"doc:/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"

The actual link we can get in RenderContentCompiler/visitLink

├─ Link destination: \"doc://org.swift.docc.example/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"
│  └─ Text \"doc:/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"

Since we saw they did not match, we'll set overrdingTitle to a non-nil value, which causes the test failure's problem.

How can we get to know such link actually should also set overrdingTitle to nil? 🤔

A simple solution is to store the original destination in Markdown/Link struct which may cause some performance issue. Any other suggestions on this? cc @franklinsch

See more info here #376

[franklinsch]

But this will cause some tests to fail and maybe break some original expected behavior.

Can you provide more details about how the current behavior is impacted from an end user perspective? Or is the change in behavior not user facing? It would be useful to see what the render JSON would look like for a specific link before and after your change.

How can we get to know such link actually should also set overridingTitle to nil?

From a render JSON perspective, it seems like we should only set an overridingTitle when the Markdown content has a title override, e.g., using []() rather than <>, but maybe I'm missing an edge case regarding tutorials?

[Kyle-Ye]

But this will cause some tests to fail and maybe break some original expected behavior.

Can you provide more details about how the current behavior is impacted from an end user perspective? Or is the change in behavior not user facing? It would be useful to see what the render JSON would look like for a specific link before and after your change.

How can we get to know such link actually should also set overridingTitle to nil?

From a render JSON perspective, it seems like we should only set an overridingTitle when the Markdown content has a title override, e.g., using []() rather than <>, but maybe I'm missing an edge case regarding tutorials?

When we consume the link, we can't get its original presentation ([]() or <>).

Original link

├─ Link destination: \"doc:/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-> %F0%9F%92%BB\"
│  └─ Text \"doc:/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"

The actual link we can get in RenderContentCompiler/visitLink

├─ Link destination: \"doc://org.swift.docc.example/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"
│  └─ Text \"doc:/tutorials/Test-Bundle/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB\"

So if we use let overridingTitle = plainTextLinkTitle == destination ? nil : plainTextLinkTitle , it will make link in tutorial fail because in tutorial it will append the identifier to its link destination.

Source:

@Section(title: "Create a New AR Project 💻") {
      @ContentAndMedia {
         This section link refers to this section itself: <doc:/TestTutorial#Create-a-New-AR-Project-%F0%9F%92%BB>.
      }
}

Before

After

[Kyle-Ye]

When we consume the link, we can't get its original presentation ( or <>).

Tested on swift-markdown:

let string = """
[custom title](doc:/Text)

[doc:/Test](doc:/Text)

<doc:/Text>
"""

let expectedDump = """
Document @1:1-5:12
├─ Paragraph @1:1-1:26
│  └─ Link @1:1-1:26 destination: "doc:/Text"
│     └─ Text @1:2-1:14 "custom title"
├─ Paragraph @3:1-3:23
│  └─ Link @3:1-3:23 destination: "doc:/Text"
│     └─ Text @3:2-3:11 "doc:/Test"
└─ Paragraph @5:1-5:12
   └─ Link @5:1-5:12 destination: "doc:/Text"
      └─ Text @5:2-5:11 "doc:/Text"
"""

[Kyle-Ye]

Two possible fixes for me:

1. Add a new property to Link so that we can differentiate []() and <doc:> in the consume site. cc @QuietMisdreavus

Update:

Or use a tricky way to differentiate it (eg. if link.range is link.firstChild.range +1)

But we've lost the range info since we altered the destination on Link when visiting link.

https://github.com/apple/swift-docc/blob/a3c31ee5dbbf5e900a56656f4e933c7b4da9795a/Sources/SwiftDocC/Semantics/MarkupReferenceResolver.swift#L116-L118

Add a PR to persevere such information swiftlang/swift-markdown#82

2. Continue to add more ifs on such context or doc:// to the original if logic. (NOT RECOMMENDED and there may be some false positive)
   cc @d-ronnqvist (Reviewer of Fix custom doc link title issue #376 )

[QuietMisdreavus]

This still needs #376 to land before it's fully fixed; i'll handle merging it.