Skip to content

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
wants to merge 74 commits into
base: main
Choose a base branch
from
Draft

Diff starter #2

wants to merge 74 commits into from

Conversation

mayaepps
Copy link
Owner

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.

  • [n/a ] Added tests -- I have not written tests for this yet since it's changing so often
  • Ran the ./bin/test script and it succeeded
  • [n/a] Updated documentation if necessary

ethan-kusters and others added 21 commits May 19, 2022 14:06
… 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
@mayaepps mayaepps requested a review from ethan-kusters May 31, 2022 22:56
@mayaepps mayaepps self-assigned this May 31, 2022
Kyle-Ye and others added 7 commits June 1, 2022 22:43
* 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
Maya Epps added 30 commits June 10, 2022 16:19
… 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
… ExpressibleByArgument conformance from DocCArchiveOption
…from the old archive, adds diffing against any RenderNode already there when writing RenderNode to JSON
…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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants