Handle Markdown and Tutorial files in textDocument/doccDocumentation #1959
Conversation
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
@swift-ci please test windows platform |
matthewbastien
force-pushed
the
markdown-documentation
branch
from
aa29b89 to
2530549
Compare
|
@swift-ci please test |
|
@swift-ci please test windows platform |
There was a problem hiding this comment.
Thanks for working on this @matthewbastien 🙏🏽
I left some comments inline. On a layering level, I think we should not build the DocCDocumentation module at all if we are building SourceKit-LSP without docc support. That way DocCDocumentation doesn’t need any #if statements and we should be able to eliminate all the protocols. Instead the code in the SourceKitLSP module would need to be guarded by #if statements.
Sources/SourceKitLSP/Documentation/DoccDocumentationHandler.swift
Outdated
Show resolved
Hide resolved
| data: try JSONEncoder().encode( | ||
| SymbolGraph( | ||
| metadata: SymbolGraph.Metadata( | ||
| formatVersion: SymbolGraph.SemanticVersion(major: 0, minor: 5, patch: 0), |
There was a problem hiding this comment.
No reason in particular. It's the version sourcekitd was reporting in its symbol graph. I'm not sure how relevant that version number is to SwiftDocC.
Sources/DocCDocumentation/BuildSystemIntegrationExtensions.swift
Outdated
Show resolved
Hide resolved
| var doccCatalogURL: URL? { | ||
| var pathComponents = self.pathComponents | ||
| var result = self | ||
| while let lastPathComponent = pathComponents.last { | ||
| if lastPathComponent.hasSuffix(".docc") { | ||
| return result | ||
| } | ||
| pathComponents.removeLast() | ||
| result.deleteLastPathComponent() | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
This would return incorrect results if the user chose to put their package in a directory that ends with .docc, right? I mean, questionable choice but it would be better to guard against that. What do you think about extending the isHeader filed in SourceKitSourceItemData to a kind filed, which can then be normal source file, header, or docc catalog?
When deserializing, I think it would be good to continue supporting the isHeader field for a while for backwards compatibility but within SourceKit-LSP, we could use that kind.
There was a problem hiding this comment.
Good idea. I added the relevant code to SourceKitSourceItemData. LMK if it functions as you expected as I'm not all that familiar with that section of the code base.
| let snapshot = try await self.latestSnapshot(for: uri) | ||
| let snapshot = try await self.latestSnapshotOrDisk(for: uri) |
There was a problem hiding this comment.
This isn’t great because it means that we can send cursor info requests to sourcekitd when the document isn’t open. Today, sourcekitd falls back to reading the file on disk if it isn’t open but I don’t want to add more reliance on this.
Also, this reminds me of an another problem: The location you get from the index will always be from the last time that the index was updated. Even if indexing was instant, this will be the last time the file was saved, but we’re now performing cursor info on the edited version of the document. We need to either:
- Not rely on the index to find the location of the symbol, which will be tricky
- Open a separate helper document in sourcekitd that matches the on-disk state, similar to what we used to do here:
sourcekit-lsp/Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift
Lines 664 to 669 in b555f5a
There was a problem hiding this comment.
I went with the helper document here to ensure that sourcekitd knows of the most recent file contents. We'll have to think a bit more about how we can handle an out of date index. FWIW in my testing I never encountered any issues using the position, though I was using a pretty simple project.
| // Do a lookup to find the top level symbol | ||
| let topLevelSymbolName = components.removeLast().name | ||
| var topLevelSymbolOccurrences = [SymbolOccurrence]() | ||
| index.forEachCanonicalSymbolOccurrence(byName: topLevelSymbolName) { symbolOccurrence in |
There was a problem hiding this comment.
How do we resolve overloads here? Eg. if you have foo(x: Int) and foo(y: Int) in your module, how do we know which one to pick?
|
@swift-ci please test |
matthewbastien
force-pushed
the
markdown-documentation
branch
from
bbaa6e3 to
6306675
Compare
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
matthewbastien
force-pushed
the
markdown-documentation
branch
from
22842b3 to
2bdc4c8
Compare
|
@swift-ci please test |
|
@swift-ci please test windows platform |
matthewbastien
force-pushed
the
markdown-documentation
branch
2 times, most recently
from
bac8896 to
c75e2ab
Compare
matthewbastien
force-pushed
the
markdown-documentation
branch
from
c75e2ab to
3da95ef
Compare
|
@swift-ci please test |
|
@swift-ci please test windows platform |
Adds a handler to
DocumentationLanguageServicefor thetextDocument/doccDocumentationLSP extension. The request is handled much in the same way as forSwiftLanguageServicewhere the SwiftDocC convert request is assembled and then sent to SwiftDocC for processing. All of the SwiftDocC specific code has been moved into a newDocCDocumentationmodule to try and keep SourceKit-LSP clean.SwiftDocC supports four different kinds of Markdown/Tutorial files:
*.tutorialfile extension and represents a self-contained tutorial.*.tutorialfile extension, but represents a group of tutorials.In addition to finding the Swift symbol for a symbol extension page, we must do the reverse when generating documentation for a symbol within a Swift file. This involves looking for
*.doccdocumentation catalogs within the target and asking SwiftDocC to build an index for us. We can then retrieve all of the markdown extension pages and their associated symbols to do the lookup.