Add new DisplayName directive to customize symbol names #92

d-ronnqvist · 2022-02-19T01:19:53Z

Bug/issue #, if applicable: SR-15572 rdar://72969138

Summary

This adds a new @DisplayName directive to allow for customization of symbol display names, as described in this forums post.

There are two pieces that don't work like what's described in that forums post yet:

Customizing the display name of a module symbol doesn't change how that module is listed as a Technology, neither for the module symbol itself nor for other symbols in that module.
Documentation without a docc catalog doesn't leverage this directive yet to customize the display name based on the command line display name argument.

Dependencies

This depends on swiftlang/swift-markdown#27

Testing

With a documentation catalog

In a documentation catalog with symbol graph files and documentation extension files:

Preview the documentation for that docc catalog with docc preview /path/to/Example.docc
In the documentation extension file for the module symbol: add a display name directive. For example:

@Metadata {
  @DisplayName("Test name")
}

Navigate to that page in the preview. The custom name ("Test name") should appear as the title, technology name, and breadcrumb for that symbol.

Navigate to a child symbol within that technology. The custom name ("Test name") should appear as the technology name and in the breadcrumbs for that symbol.

Without a documentation catalog

With a collection of symbol graph files:

Preview the documentation and pass a custom display name on the command line:

docc \
 --fallback-display-name "Test name again" \
 --additional-symbol-graph-dir /path/to/directory-of-symbol-graph-files/

Navigate to that page in the preview. The custom name ("Test name again") should appear as the title, technology name, and breadcrumb for that symbol.

Navigate to a child symbol within that technology. The custom name ("Test name again") should appear as the technology name and in the breadcrumbs for that symbol.

Checklist

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

Added tests
Ran the ./bin/test script and it succeeded
Updated documentation if necessary

rdar://72969138

…ntation

Also, remove language variance for module name related properties.

d-ronnqvist · 2022-03-02T02:46:48Z

Sources/SwiftDocC/Model/DocumentationNode.swift

@@ -148,7 +148,8 @@ public struct DocumentationNode {
    /// > Warning: Using this initializer will not report any diagnostics raised during the initialization of `DocumentationNode`.
    @available(*, deprecated, message: "Use init(reference:symbol:platformName:moduleName:article:engine:) instead")
    public init(reference: ResolvedTopicReference, symbol: SymbolGraph.Symbol, platformName: String?, moduleName: String, article: Article?, problems: inout [Problem]) {


It would be good if to remove this initializer. It's been deprecated for a while now.

franklinsch · 2022-03-02T09:40:15Z

Package.resolved

-          "branch": "main",
-          "revision": "9a9050095ec93853c0ed01930a9d108d79dc4776",
+          "branch": "unlabeled-directive-argument",
+          "revision": "5cb5ae2c69fe2d0d490fe62fb119bd724fd98ac6",


What's your plan for landing these changes? It doesn't look like we're introducing new API in that branch that we're using in DocC, but I assume that some tests will fail without the Swift-Markdown changes?

I plan on landing the Swift-Markdown changes first and then this. Without the Swift-Markdown changes, this new directive (and the tests that make use of it) will encounter parsing errors.

franklinsch · 2022-03-02T10:24:58Z

Sources/SwiftDocC/Infrastructure/Workspace/GeneratedDataProvider.swift

@@ -12,11 +12,12 @@ import Foundation
 import SymbolKit

 /// A type that provides documentation bundles that it discovers by traversing the local file system.
-public struct GeneratedDataProvider: DocumentationWorkspaceDataProvider {
+public class GeneratedDataProvider: DocumentationWorkspaceDataProvider {


Can you elaborate here? Switching this to a class makes this type harder to use in concurrent environments. Also, might want to make this final.

Yes. This is driven by needing to know what data to return from contentsOfURL. That data depend on if the generated bundle contains one or more modules. However, the argument to this method doesn't indicate one or the other so the number of modules (or its derivate values) need to be stored as private state. The information needed to know if the bundle represents one or more modules isn't known until bundles(options:) is called so it can't be computed during initialization.

Given the choice between making one provider a class or making bundles(options:) a mutating func I went with the former because it was a smaller change but I can see the argument for making mutation this a formal part of the provider API.

In practice, a provider is only asked to provide its bundles once and is only asked to provide content for urls in the returned bundle, so this won't have any impact on concurrency. If a developer did set up a shared GeneratedDataProvider with more than one documentation context, the real issue would be calling bundles(options:) twice.

This class isn't open, so making it final would only make a difference within Swift-DocC. I don't think we have a case for subclassing this but we don't strictly have a case for not allowing subclassing within Swift-DocC either.

franklinsch · 2022-03-02T14:05:43Z

Sources/SwiftDocC/Model/DocumentationNode.swift

-        }
+        let extendedModule: String? = unifiedSymbol.mixins
+            .first(where: { $0.0.platform == platformName })?.value
+            .getValueIfPresent(for: SymbolGraph.Symbol.Swift.Extension.self)?.extendedModule


Not sure I understand this change, can you elaborate? Should we not traverse the mixins for each language variant the symbol is available in?

From what I understand, module names are required to be the same in all languages. The Symbol semantic no longer stores variants of module related information so this computes a single language agnostic value.

I think that makes sense, because the UnifiedSymbolGraph logic relies on the symbol graph files for each language variant of a module to be the same, right @QuietMisdreavus?

franklinsch · 2022-03-02T14:07:00Z

Sources/SwiftDocC/Semantics/Article/Article.swift

+        let isDocumentationExtension = title.child(at: 0) is AnyLink
+        if !isDocumentationExtension, let metadata = optionalMetadata, let displayName = metadata.displayName {
+            let diagnosticSummary = """
+            A \(DisplayName.directiveName.singleQuoted) directive is only supported in documentation extensions files. To customize the display name of an article, change the content of the level-1 heading.


Suggested change

A \(DisplayName.directiveName.singleQuoted) directive is only supported in documentation extensions files. To customize the display name of an article, change the content of the level-1 heading.

A \(DisplayName.directiveName.singleQuoted) directive is only supported in documentation extension files. To customize the display name of an article, change the content of the level-1 heading.

franklinsch · 2022-03-02T14:07:11Z

Sources/SwiftDocC/Semantics/Article/Article.swift

+            let solutions: [Solution]
+            if let displayNameRange = displayName.originalMarkup.range, let titleRange = title.range {
+                let removeDisplayNameReplacement = Replacement(range: displayNameRange, replacement: "")
+                let changeTitleReplacement = Replacement(range: titleRange, replacement: "# \(displayName.name)")


franklinsch · 2022-03-02T14:10:59Z

Sources/SwiftDocC/Infrastructure/DocumentationContext.swift

-
+
+    /// A cache of plain string module names, keyed by the module node reference.
+    private var moduleNameCache: [ResolvedTopicReference: String] = [:]


Not sure I understand the need for this cache, rather than just using the node's title. Is this for performance?

Yes, this is only for performance. The module symbol's name will be displayed on all symbol's pages so it's a good candidate for a value to cache.

Ok, that makes sense. Wondering if we should make this an IOU so that we don't accidentally access it before it's been populated. We do the same for DocumentationContext.rootModules.

I think that's a good idea. I made this non-optional and added asserts about how this API is used.

d-ronnqvist · 2022-03-02T22:14:12Z

@swift-ci please test

- Assert if module names are not found - Fix spelling in Article diagnostic message

d-ronnqvist · 2022-03-02T23:11:37Z

@swift-ci please test

d-ronnqvist · 2022-03-03T17:15:31Z

@swift-ci please test

d-ronnqvist · 2022-03-03T17:28:58Z

@swift-ci please test

d-ronnqvist added 6 commits February 18, 2022 16:58

Add a new DisplayName directive to customize symbol names

94e7d82

rdar://72969138

Merge branch 'main' into customize-symbol-display-name

ca91056

Use DisplayName directive to customize generated single module docume…

ab8ea93

…ntation

Update test to replace use of deprecated documentation context API

e6b9294

Replace moduleName with moduleReference.

797f7de

Also, remove language variance for module name related properties.

Merge branch 'main' into customize-symbol-display-name

74f7dd1

d-ronnqvist commented Mar 2, 2022

View reviewed changes

Add test assertion for the rendered module name metadata

29a7ace

d-ronnqvist marked this pull request as ready for review March 2, 2022 02:55

d-ronnqvist requested review from franklinsch and ethan-kusters March 2, 2022 02:55

Add back missing default availability rendering logic

50b8f82

franklinsch reviewed Mar 2, 2022

View reviewed changes

franklinsch approved these changes Mar 2, 2022

View reviewed changes

Address PR feedback:

609e200

- Assert if module names are not found - Fix spelling in Article diagnostic message

d-ronnqvist force-pushed the customize-symbol-display-name branch from 3ff4c32 to 609e200 Compare March 2, 2022 23:03

d-ronnqvist added 2 commits March 3, 2022 09:14

Use merged commit of swift-markdown

9c6483c

Merge branch 'main' into customize-symbol-display-name

0ab9bd1

d-ronnqvist added 2 commits March 3, 2022 09:28

Update new test to use new documentation node initializer

8ecaf41

Restore previous documentation node initializer as deprecated

27c3692

d-ronnqvist merged commit 0661e96 into swiftlang:main Mar 3, 2022

d-ronnqvist mentioned this pull request Feb 19, 2022

[SR-15572] [Swift-DocC] Support custom display names for frameworks #199

Closed

ethan-kusters mentioned this pull request May 10, 2022

Avoid using the module's customized display name when creating availability render items #161

Merged

3 tasks

d-ronnqvist deleted the customize-symbol-display-name branch September 11, 2023 22:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new DisplayName directive to customize symbol names #92

Add new DisplayName directive to customize symbol names #92

d-ronnqvist commented Feb 19, 2022 •

edited

Loading

d-ronnqvist Mar 2, 2022

franklinsch Mar 2, 2022

d-ronnqvist Mar 2, 2022

franklinsch Mar 2, 2022

d-ronnqvist Mar 2, 2022

franklinsch Mar 2, 2022

d-ronnqvist Mar 2, 2022

franklinsch Mar 2, 2022

franklinsch Mar 2, 2022

franklinsch Mar 2, 2022

franklinsch Mar 2, 2022

d-ronnqvist Mar 2, 2022

franklinsch Mar 2, 2022

d-ronnqvist Mar 2, 2022

d-ronnqvist commented Mar 2, 2022

d-ronnqvist commented Mar 2, 2022

d-ronnqvist commented Mar 3, 2022

d-ronnqvist commented Mar 3, 2022

	A \(DisplayName.directiveName.singleQuoted) directive is only supported in documentation extensions files. To customize the display name of an article, change the content of the level-1 heading.
	A \(DisplayName.directiveName.singleQuoted) directive is only supported in documentation extension files. To customize the display name of an article, change the content of the level-1 heading.



		/// A cache of plain string module names, keyed by the module node reference.
		private var moduleNameCache: [ResolvedTopicReference: String] = [:]

Add new DisplayName directive to customize symbol names #92

Add new DisplayName directive to customize symbol names #92

Conversation

d-ronnqvist commented Feb 19, 2022 • edited Loading

Summary

Dependencies

Testing

With a documentation catalog

Without a documentation catalog

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-ronnqvist commented Mar 2, 2022

d-ronnqvist commented Mar 2, 2022

d-ronnqvist commented Mar 3, 2022

d-ronnqvist commented Mar 3, 2022

d-ronnqvist commented Feb 19, 2022 •

edited

Loading