-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Refactor diff viewer onto Pierre React primitives #5308
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
base: main
Are you sure you want to change the base?
Changes from 27 commits
6efaa2c
8bb8098
e32358b
2264854
1d0c0fe
438560d
a4624e5
1aadcef
846934a
4ba4b0c
51ee179
19d1351
304cd79
4d945c3
e8bd0d5
0ad95e9
d73885b
066c501
9cf96b4
ae6033e
287d9ac
03dfd5b
74d571e
7454c5b
ab900fd
ffe6549
a1148de
e984010
f1c0072
ac24014
01a7d6b
b8d7c6b
8ea122d
5a8a920
50e0d44
62e8d51
87f9558
3213c1b
dbe7678
9b65c3c
19351af
d648439
86a5a07
eeead53
9913209
c46565f
f7c1525
5b7f9ce
528fe3d
2ae83fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -402,6 +402,7 @@ extension CMUXCLI { | |
| "commit": CMUXDiffViewerLocalization.string("about.commit", defaultValue: "Commit"), | ||
| "collapseAllDiffs": CMUXDiffViewerLocalization.string("diffViewer.collapseAllDiffs", defaultValue: "Collapse all diffs"), | ||
| "collapseUnchangedContext": CMUXDiffViewerLocalization.string("diffViewer.collapseUnchangedContext", defaultValue: "Collapse unchanged context"), | ||
| "copyFailedGitApplyCommand": CMUXDiffViewerLocalization.string("diffViewer.copyFailedGitApplyCommand", defaultValue: "Could not copy git apply command."), | ||
| "copiedGitApplyCommand": CMUXDiffViewerLocalization.string("diffViewer.copiedGitApplyCommand", defaultValue: "Copied git apply command"), | ||
| "copyGitApplyCommand": CMUXDiffViewerLocalization.string("diffViewer.copyGitApplyCommand", defaultValue: "Copy git apply command"), | ||
| "deletions": CMUXDiffViewerLocalization.string("diffViewer.deletions", defaultValue: "Deletions"), | ||
|
|
@@ -1362,23 +1363,63 @@ extension CMUXCLI { | |
|
|
||
| if host == "diffshub.com" || host == "www.diffshub.com" { | ||
| let components = url.pathComponents | ||
| if components.count >= 5, | ||
| components[3] == "pull", | ||
| Int(components[4]) != nil { | ||
| return URL(string: "https://github.com/\(components[1])/\(components[2])/pull/\(components[4]).diff") | ||
| guard components.count >= 5 else { | ||
| return url | ||
| } | ||
| if components[3] == "pull" { | ||
| return trustedGitHubPullPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| pullComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "compare" { | ||
| return trustedGitHubComparePatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| compareComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "commit" { | ||
| return trustedGitHubCommitPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| commitComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if host == "github.com" || host == "www.github.com" { | ||
| let components = url.pathComponents | ||
| if components.count >= 5, | ||
| components[3] == "pull", | ||
| Int(components[4].replacingOccurrences(of: ".patch", with: "").replacingOccurrences(of: ".diff", with: "")) != nil { | ||
| let pullComponent = components[4] | ||
| if pullComponent.hasSuffix(".patch") || pullComponent.hasSuffix(".diff") { | ||
| return url | ||
| } | ||
| return URL(string: "https://github.com/\(components[1])/\(components[2])/pull/\(pullComponent).diff") | ||
| guard components.count >= 5 else { | ||
| return url | ||
| } | ||
| if components[3] == "pull" { | ||
| return trustedGitHubPullPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| pullComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "compare" { | ||
| return trustedGitHubComparePatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| compareComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
| if components[3] == "commit" { | ||
| return trustedGitHubCommitPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| commitComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1395,30 +1436,66 @@ extension CMUXCLI { | |
|
|
||
| if host == "diffshub.com" || host == "www.diffshub.com" { | ||
| let components = url.pathComponents | ||
| guard components.count >= 5, | ||
| components[3] == "pull" else { | ||
| guard components.count >= 5 else { | ||
| return nil | ||
| } | ||
| return trustedGitHubPullPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| pullComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| if components[3] == "pull" { | ||
| return trustedGitHubPullPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| pullComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "compare" { | ||
| return trustedGitHubComparePatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| compareComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "commit" { | ||
| return trustedGitHubCommitPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| commitComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| if host == "github.com" || host == "www.github.com" { | ||
| let components = url.pathComponents | ||
| guard components.count >= 5, | ||
| components[3] == "pull" else { | ||
| guard components.count >= 5 else { | ||
| return nil | ||
| } | ||
| return trustedGitHubPullPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| pullComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| if components[3] == "pull" { | ||
| return trustedGitHubPullPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| pullComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "compare" { | ||
| return trustedGitHubComparePatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| compareComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| if components[3] == "commit" { | ||
| return trustedGitHubCommitPatchURL( | ||
| owner: components[1], | ||
| repo: components[2], | ||
| commitComponent: components[4], | ||
| defaultExtension: "diff" | ||
| ) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| return nil | ||
|
|
@@ -1455,6 +1532,71 @@ extension CMUXCLI { | |
| return URL(string: "https://github.com/\(owner)/\(repo)/pull/\(pullNumber).\(suffix)") | ||
| } | ||
|
|
||
| private func trustedGitHubComparePatchURL( | ||
| owner: String, | ||
| repo: String, | ||
| compareComponent: String, | ||
| defaultExtension: String | ||
| ) -> URL? { | ||
| guard githubPathSegmentIsSafe(owner), | ||
| githubPathSegmentIsSafe(repo) else { | ||
| return nil | ||
| } | ||
|
|
||
| let suffix: String | ||
| let compareSpec: String | ||
| if compareComponent.hasSuffix(".patch") { | ||
| suffix = "patch" | ||
| compareSpec = String(compareComponent.dropLast(".patch".count)) | ||
| } else if compareComponent.hasSuffix(".diff") { | ||
| suffix = "diff" | ||
| compareSpec = String(compareComponent.dropLast(".diff".count)) | ||
| } else { | ||
| suffix = defaultExtension | ||
| compareSpec = compareComponent | ||
| } | ||
| guard suffix == "diff" || suffix == "patch", | ||
| githubCompareSpecIsSafe(compareSpec) else { | ||
| return nil | ||
| } | ||
| return URL(string: "https://github.com/\(owner)/\(repo)/compare/\(compareSpec).\(suffix)") | ||
| } | ||
|
|
||
| private func trustedGitHubCommitPatchURL( | ||
| owner: String, | ||
| repo: String, | ||
| commitComponent: String, | ||
| defaultExtension: String | ||
| ) -> URL? { | ||
| guard githubPathSegmentIsSafe(owner), | ||
| githubPathSegmentIsSafe(repo) else { | ||
| return nil | ||
| } | ||
|
|
||
| let suffix: String | ||
| let commit: String | ||
| if commitComponent.hasSuffix(".patch") { | ||
| suffix = "patch" | ||
| commit = String(commitComponent.dropLast(".patch".count)) | ||
| } else if commitComponent.hasSuffix(".diff") { | ||
| suffix = "diff" | ||
| commit = String(commitComponent.dropLast(".diff".count)) | ||
| } else { | ||
| suffix = defaultExtension | ||
| commit = commitComponent | ||
| } | ||
| guard suffix == "diff" || suffix == "patch", | ||
| commit.count >= 7, | ||
| commit.unicodeScalars.allSatisfy({ scalar in | ||
| (scalar.value >= 48 && scalar.value <= 57) || | ||
| (scalar.value >= 65 && scalar.value <= 70) || | ||
| (scalar.value >= 97 && scalar.value <= 102) | ||
| }) else { | ||
| return nil | ||
| } | ||
| return URL(string: "https://github.com/\(owner)/\(repo)/commit/\(commit).\(suffix)") | ||
| } | ||
|
|
||
| private func githubPathSegmentIsSafe(_ component: String) -> Bool { | ||
| guard !component.isEmpty else { return false } | ||
| return component.unicodeScalars.allSatisfy { scalar in | ||
|
|
@@ -1467,6 +1609,19 @@ extension CMUXCLI { | |
| } | ||
| } | ||
|
|
||
| private func githubCompareSpecIsSafe(_ component: String) -> Bool { | ||
| guard component.contains(".."), !component.isEmpty else { return false } | ||
| return component.unicodeScalars.allSatisfy { scalar in | ||
| (scalar.value >= 48 && scalar.value <= 57) || | ||
| (scalar.value >= 65 && scalar.value <= 90) || | ||
| (scalar.value >= 97 && scalar.value <= 122) || | ||
| scalar == "-" || | ||
| scalar == "_" || | ||
| scalar == "." || | ||
| scalar == "/" | ||
| } | ||
| } | ||
|
|
||
| private func diffInputExternalURL(_ url: URL) -> URL { | ||
| guard let host = url.host?.lowercased(), | ||
| host == "github.com" || host == "www.github.com" else { | ||
|
|
@@ -5037,7 +5192,7 @@ extension CMUXCLI { | |
| return path.hasSuffix(".html") | ||
| } | ||
| if mimeType == "text/javascript" { | ||
| return path.hasSuffix(".mjs") | ||
| return path.hasSuffix(".mjs") || path.hasSuffix(".js") | ||
| } | ||
| if mimeType == "text/x-diff" { | ||
| return path.hasSuffix(".patch") | ||
|
|
@@ -5461,7 +5616,7 @@ extension CMUXCLI { | |
|
|
||
| private func ensureDiffViewerAssets(nextTo viewerURL: URL) throws -> DiffViewerAssets { | ||
| let sourceDirectory = try diffViewerBundledAssetDirectory() | ||
| let assetDirectoryName = "pierre-diffs-1.2.1-trees-1.0.0-beta.4" | ||
| let assetDirectoryName = "pierre-diffs-1.2.7-trees-1.0.0-beta.4" | ||
| let targetDirectory = viewerURL.deletingLastPathComponent() | ||
| .appendingPathComponent("assets", isDirectory: true) | ||
| .appendingPathComponent(assetDirectoryName, isDirectory: true) | ||
|
|
@@ -5478,7 +5633,7 @@ extension CMUXCLI { | |
| guard assetPaths.contains("diffs.mjs"), | ||
| assetPaths.contains("trees.mjs"), | ||
| assetPaths.contains("worker-pool/worker-pool.mjs"), | ||
| assetPaths.contains("worker-pool/worker-portable.mjs") else { | ||
| assetPaths.contains("worker-pool/worker-portable.js") else { | ||
| throw CLIError(message: "Bundled diff viewer entry assets not found") | ||
| } | ||
| for assetPath in assetPaths { | ||
|
|
@@ -5498,7 +5653,7 @@ extension CMUXCLI { | |
| diffsModuleURL: "./assets/\(assetDirectoryName)/diffs.mjs", | ||
| treesModuleURL: "./assets/\(assetDirectoryName)/trees.mjs", | ||
| workerPoolModuleURL: "./assets/\(assetDirectoryName)/worker-pool/worker-pool.mjs", | ||
| workerModuleURL: "./assets/\(assetDirectoryName)/worker-pool/worker-portable.mjs", | ||
| workerModuleURL: "./assets/\(assetDirectoryName)/worker-pool/worker-portable.js", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the diff viewer is restored through Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in current head. The custom-scheme handler now accepts text/javascript for both .mjs and .js paths, so worker-portable.js can load after restore. — Claude Code
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already covered in the current code: BrowserPanel.pathExtensionMatchesMimeType accepts text/javascript for both .mjs and .js, so worker-portable.js is allowed by the custom-scheme handler. — Claude Code |
||
| files: assetPaths.map { targetDirectory.appendingPathComponent($0, isDirectory: false) } | ||
| + appAssetPaths.map { targetAppDirectory.appendingPathComponent($0, isDirectory: false) } | ||
| ) | ||
|
|
@@ -5567,7 +5722,7 @@ extension CMUXCLI { | |
|
|
||
| var relativePaths: [String] = [] | ||
| for case let fileURL as URL in enumerator { | ||
| guard fileURL.pathExtension == "mjs", | ||
| guard ["js", "mjs"].contains(fileURL.pathExtension), | ||
| let values = try? fileURL.resourceValues(forKeys: [.isRegularFileKey]), | ||
| values.isRegularFile == true else { | ||
| continue | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.