forked from swiftlang/swift-docc
-
Notifications
You must be signed in to change notification settings - Fork 0
Diff starter #2
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
Draft
mayaepps
wants to merge
74
commits into
main
Choose a base branch
from
diff-starter
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Diff starter #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… SemanticVersion, Dictionary, TopicRenderReference are all Diffable
Begins to resolve a significant (~40%) performance regression caused by enabling static hosting transformation by default during `docc convert`. The strategy here is to integrate the static hosting transformation into the existing work being done to write Render JSON files to disk. This has two immediate benefits: 1. The creation of the index.html files will be done in parallel since the RenderJSON creation is already parallelized. 2. It removes the need to re-walk the file system to determine the correct paths to write the 'index.html' files based on where Render JSON files are written. Instead `docc convert` just creates two files where it used to create a single Render JSON file. Note that this doesn't _fully_ resolve the ~40% regression as there's still additional filesystem work being done. There will be follow-up commits to try to find other performance wins in Swift-DocC to address this.
The current implementation fully loads the contents of each image file into memory as part of checking to see if a reference to an image is valid. This adds a new `resourceExists(with:)` method to `DocumentationContext` so that it can be used instead of `DocumentationContest.resource(with:) != nil`.
Add missing `isDirectory` values in `appendingPathComponent` use URL can perform disk access to determine if the given path component points to a directory if this value is not specified so it can become a performance issue.
By default Swift-DocC writes all files to disk atomically. This is generally a good default since it can prevent confusing errors. When writing atomically, the file will first be written to a temp location and then, only when the writing is complete, copied into the final destination. This means that if writing fails halfway through, there won't be a malformed file in the destination. If the file is there at all, it's correct. However, for writing RenderJSON from `docc convert` specifically, this is unnecessary for a couple of reasons. 1. The `docc convert` process never reads RenderJSON, only writes it. 2. `docc convert` already writes _all_ of its output to a temp directory and only copies it over to the final destination if the entire conversion ran without errors. So in the event that a RenderJSON write _does_ fail halfway through, we still end up with the desired behavior of not having malformed JSON in the target destination. An error would be thrown and the conversion would fail without copying any of the bad output from the temp directory. As it stands `docc convert` effectively implements atomic writes twice. The reason to _not_ write RenderJSON atomically is performance. By writing non-atomically there's an ~8% performance win for the "documentation-processing" step of the conversion process.
The primary performance win here comes from updating the logic that creates a normalized navigator index identifier from a ResolvedTopicReference to not rely on creating an additional ResolvedTopicReference.
Updates swift-markdown to include: commit 97df6e2812adcf8698204ca5f0756563ef36e5c1 Author: Ethan Kusters <ekusters@apple.com> Date: Sat May 21 14:02:22 2022 -0700 Correctly set `indexInParent` in `Markup.child(at:)` (swiftlang#45) This fixes a recent regression where `Markup.child(at:)` began returning markup with incorrect metadata resulting in out-of-bounds errors. The `indexInParent` value of a child is unrelated to the parent's `indexInParent`. This was missed initially because tests were only checking for children of the first item where `indexInParent` would be 0. A new test has been added that asserts `Markup.child(at:)` returns correct values for children of nested items as well. commit 7a7c59d1160fddd23e2379ec5ee8c71df7fd0b3b Author: Ethan Kusters <ekusters@apple.com> Date: Fri May 20 18:53:24 2022 -0700 Improve performance of `Markup.child(at:)` method (swiftlang#44) Improves the performance of `Markup.child(at:)` by refactoring to removing the need to iterate over all previous elements in the child array. commit 36cf89e36e94b025dbe37d82fe2a7da3f39d4204 Author: Franklin Schrans <fschrans@apple.com> Date: Wed Mar 23 14:38:18 2022 +0100 Remove extraneous print in test (swiftlang#33) commit e50693584310a9190071c96c839485b6f2376832 Author: Ethan Kusters <ekusters@apple.com> Date: Mon Mar 21 10:42:02 2022 -0700 Publish Swift Markdown's documentation to GitHub pages (swiftlang#32) * Adopt the Swift-DocC Plugin for documentation generation * Add a script for publishing docs to GitHub pages * Add missing license headers * Move README docs to articles in the DocC catalog * Remove out-of-date documentation about `DiagnosticEngine` Swift markdown no longer includes a DiagnosticEngine so these links were failing to resolve. commit 5f10cfb030f43222a49ea34857ff07f02f1e38b0 Author: christopherweems <github@christopherweems.com> Date: Sat Mar 19 12:53:31 2022 -0400 Fix typo in documentation for `MarkupVisitor` (swiftlang#26) Co-authored-by: Christopher Weems <hello@christopherweems.com>
…tlang#262) * Allow writing doc links to subsections using special characters rdar://93458581 * Rename ValidatedURL initializers to indicate intended use at call site
…et diffable working with protocols unsuccessfully
This stores a handle to the attributes in the index, the tree, and the individual nodes. Attributes are stored in a `[String: Any]` and the client is expected to provided extensions with computed properties as needed. Additionaly this commits refactors the API for reading a NavigatorIndex from an on-disk LMDB database. The prveious API used a designated initializer to do this. However, this initializer was complex as it opened database connections etc... Using a static function enables clients to be more resilient to API changes, by letting them define protocols that specify the new API and providing a no-op default implementation that delegates to old API. This is not currently possible with the initializer base API as the initializer would need to be marked required to ensure subclasses of NavigatorIndex implement this. This will be made possible in the future as clients transition to the static function based API. rdar://93503303
* Use `python3` in build-script-helper Python2 is no longer supported by the Swift project. * Remove unused variable from build script `output_dir` is written to but never used.
…derNodes as JSONPatches. Also adds the old RenderNode being compared and the patches being accumulated to the UserInfo in the encoder.
…vedTopicReference to their respective structs, their diffs can be encoded in JSON
…create CodingKeys for each key
… SemanticVersion, Dictionary, TopicRenderReference are all Diffable
…et diffable working with protocols unsuccessfully
…derNodes as JSONPatches. Also adds the old RenderNode being compared and the patches being accumulated to the UserInfo in the encoder.
…vedTopicReference to their respective structs, their diffs can be encoded in JSON
…create CodingKeys for each key
…n the command line
…c to validate the previous archive path
… ExpressibleByArgument conformance from DocCArchiveOption
…les don't get dropped
…from the old archive, adds diffing against any RenderNode already there when writing RenderNode to JSON
… versions to RenderNode keys.
… been provided to use for diffing.
…ous archive's RenderNode metadata.
…into diff-starter
…erNode JSON if one exists where the new one will be written.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bug/issue #, if applicable:
Summary
This PR makes almost all of the RenderNode conform to Diffable, meaning there is a method called difference that returns the changes between this RenderNode and the given one by checking each properties of each struct for differences. The diffs are not yet encoded to JSON.
Testing
Currently to test this I have been manually checking two archives using the diff command line argument and passing in the paths to the old archive and then the new archive.
The differences will be printed out in the format: [path: change made (including the element changed)]
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded