Prefer non-symbols in general documentation links #594
Conversation
rdar://109583745
|
@swift-ci please test |
| if let symbolOrNonSymbolMatch = collisions.singleMatch({ ($0.node.symbol != nil) == onlyFindSymbols }) { | ||
| return symbolOrNonSymbolMatch.node |
There was a problem hiding this comment.
I'm struggling to follow this line. Could it be structured like this or is it doing something different?
| if let symbolOrNonSymbolMatch = collisions.singleMatch({ ($0.node.symbol != nil) == onlyFindSymbols }) { | |
| return symbolOrNonSymbolMatch.node | |
| if !onlyFindSymbols, let nonSymbolMatch = (collisions.singleMatch { $0.node.symbol == nil }) | |
| return nonSymbolMatch.node |
If non-symbols aren't allowed, skip all non-symbols.
Have these not already been filtered out prior to this or do we need to handle that case here?
There was a problem hiding this comment.
The only difference is that with onlyFindSymbols inside the closure it checks both ways.
For symbol links (when onlyFindSymbols is true) it checks that the match has a symbol.
For general documentation links (when onlyFindSymbols is false) it checks that the match doesn't have a symbol.
Have these not already been filtered out prior to this or do we need to handle that case here?
It is checked at the start of the search to avoid looking at the article root but as I was writing this I thought that it would be possible to find an article as a child of the module. Now I'm not sure anymore.. I'll have to try it.
There was a problem hiding this comment.
Okay sounds good. If it's possible to simplify this I think that would make it easier to follow – but not a blocker.
There was a problem hiding this comment.
I tested this and in the case where a symbol and an article have the same path, a link that's spelled with the absolute path will have a collisions here. In this case filtering out non-symbols in symbol links in both places (here and where the found node is returned) provides a better experience because it avoids the collisions (and also excludes the non-symbols from diagnostics if there were multiple symbols colliding with an article).
I updated the comment to explain all this, added a test that has this behavior, and added an extra comment that explains that check is a more compact spelling of two comparisons and an if statement.
| if let symbolOrNonSymbolMatch = collisions.singleMatch({ ($0.node.symbol != nil) == onlyFindSymbols }) { | ||
| return symbolOrNonSymbolMatch.node |
There was a problem hiding this comment.
Okay sounds good. If it's possible to simplify this I think that would make it easier to follow – but not a blocker.
Also, add test where symbol and non-symbol have same path.
|
@swift-ci please test |
|
@swift-ci Please test |
* Prefer non-symbols in general documentation links rdar://109583745 * Elaborate comment about filtering symbol and non-symbol collisions. Also, add test where symbol and non-symbol have same path.
Bug/issue #, if applicable: rdar://109583745
Summary
This restores a previous behavior from the previous link resolver where doc links prioritized non-symbol results without needing disambiguation.
Dependencies
None.
Testing
Build documentation for Swift Markdown. The links to
<doc:BlockMarkup>and<doc:InlineMarkup>should resolve to those articles without needing disambiguation.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/testscript and it succeeded