From a1a90a7d2a7f0e9fb736f840a336d450490dfc15 Mon Sep 17 00:00:00 2001 From: Pierre Jacquier Date: Thu, 2 Mar 2023 21:16:22 -0500 Subject: [PATCH] Support commit diffs (#8) * cra boilerplate * Dummy chrome extension * eslint and working url popup * content script and dummy messaging * replace .obj diffs with dummy test * comment and in-order multiple type support * get pull api data from url * README title and desc * api/elements match with filename check * github token signin signout * manifest v3, service request for CORS * working kittycad api in service worker * First real background message * Clean up, better types * Fix settings * multiservice settings * Tweaks * WIP: download file * Working downloads and kittycad conversion * Inject react, add three dependencies * Working stl canvas * primer for github-like style * Loading before model * diff colors * colorMode auto * Popup clean up * clean up * Working loading * Logos * Add GitHub CI * Working test * yarn test in ci * Little tweak * Update README * component tests * Better test * Clean up * UserCard test * working caddiff test * Note * Rename App to Settings * storage test * Clean up * Clean up content script * further content cleanup * Fix test * Little tweaks to modelview * More tests and testing * Regex fix * LFS file download test * prettier config from kittycad/website * Little tweaks * comment * log level * Tweaks * README update * more prettier * comment * Irrelevant comment * No .vscode and readme update * Remove .vscode * Package.json update after vscode removal * Working commit diff * Start cleaning up * Clean up * Add artifact upload * return when matched * Better test * Clean up * prettier * Clean up html snippet --- .github/workflows/ci.yml | 4 ++ src/chrome/background.ts | 11 ++++ src/chrome/content.ts | 92 ++++++++++++++++++------------ src/chrome/types.ts | 15 ++++- src/chrome/web.test.ts | 117 ++++++++++++++++++++++++++++++++------- src/chrome/web.ts | 63 +++++++++++++++------ 6 files changed, 228 insertions(+), 74 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97e2f81..1ffd95e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,3 +23,7 @@ jobs: - run: yarn build - run: yarn test + + - uses: actions/upload-artifact@v3 + with: + path: build diff --git a/src/chrome/background.ts b/src/chrome/background.ts index f80ff34..f96f733 100644 --- a/src/chrome/background.ts +++ b/src/chrome/background.ts @@ -4,6 +4,7 @@ import { KittycadUser, Message, MessageGetFileDiff, + MessageGetGithubCommitData, MessageGetGithubPullFilesData, MessageIds, MessageResponse, @@ -86,6 +87,16 @@ chrome.runtime.onMessage.addListener( return true } + if (message.id === MessageIds.GetGithubCommit) { + const { owner, repo, sha } = + message.data as MessageGetGithubCommitData + github.rest.repos + .getCommit({ owner, repo, ref: sha }) + .then(r => sendResponse(r.data)) + .catch(e => sendResponse(e)) + return true + } + if (message.id === MessageIds.GetGithubUser) { github.rest.users .getAuthenticated() diff --git a/src/chrome/content.ts b/src/chrome/content.ts index c98062a..7dee213 100644 --- a/src/chrome/content.ts +++ b/src/chrome/content.ts @@ -2,12 +2,11 @@ import React from 'react' import { createRoot } from 'react-dom/client' import { CadDiff } from '../components/CadDiff' import { Loading } from '../components/Loading' -import { isFilenameSupported } from './diff' -import { DiffEntry, FileDiff, Message, MessageIds, Pull } from './types' +import { Commit, DiffEntry, FileDiff, Message, MessageIds, Pull } from './types' import { - getGithubUrlParams, - getWebPullElements, - getInjectablePullElements, + getGithubPullUrlParams, + mapInjectableDiffElements, + getGithubCommitUrlParams, } from './web' // https://github.com/OctoLinker/injection @@ -15,34 +14,20 @@ import { // no ts support const gitHubInjection = require('github-injection') -async function injectPullDiff( +async function injectDiff( owner: string, repo: string, - pull: number, + sha: string, + parentSha: string, + files: DiffEntry[], document: Document ) { - const allApiFiles = await chrome.runtime.sendMessage({ - id: MessageIds.GetGithubPullFiles, - data: { owner, repo, pull }, - }) - const apiFiles = allApiFiles.filter(f => isFilenameSupported(f.filename)) - console.log(`Found ${apiFiles.length} supported files with the API`) - - const elements = getWebPullElements(document) - console.log(`Found ${elements.length} elements in the web page`) - - const injectableElements = getInjectablePullElements(elements, apiFiles) - for (const { element } of injectableElements) { + const map = mapInjectableDiffElements(document, files) + for (const { element } of map) { createRoot(element).render(React.createElement(Loading)) } - const pullData = await chrome.runtime.sendMessage({ - id: MessageIds.GetGithubPull, - data: { owner, repo, pull }, - }) - const sha = pullData.head.sha - const parentSha = pullData.base.sha - for (const { element, file } of injectableElements) { + for (const { element, file } of map) { const fileDiff = await chrome.runtime.sendMessage({ id: MessageIds.GetFileDiff, data: { owner, repo, sha, parentSha, file }, @@ -51,17 +36,54 @@ async function injectPullDiff( } } +async function injectPullDiff( + owner: string, + repo: string, + pull: number, + document: Document +) { + const files = await chrome.runtime.sendMessage({ + id: MessageIds.GetGithubPullFiles, + data: { owner, repo, pull }, + }) + const pullData = await chrome.runtime.sendMessage({ + id: MessageIds.GetGithubPull, + data: { owner, repo, pull }, + }) + const sha = pullData.head.sha + const parentSha = pullData.base.sha + await injectDiff(owner, repo, sha, parentSha, files, document) +} + +async function injectCommitDiff( + owner: string, + repo: string, + sha: string, + document: Document +) { + const commit = await chrome.runtime.sendMessage({ + id: MessageIds.GetGithubCommit, + data: { owner, repo, sha }, + }) + if (!commit.files) throw Error('Found no file changes in commit') + if (!commit.parents.length) throw Error('Found no commit parent') + const parentSha = commit.parents[0].sha + injectDiff(owner, repo, sha, parentSha, commit.files, document) +} + gitHubInjection(async () => { - const params = getGithubUrlParams(window.location.href) - if (!params) { - console.log("URL doesn't match pull request pattern.") + const url = window.location.href + const pullParams = getGithubPullUrlParams(url) + if (pullParams) { + const { owner, repo, pull } = pullParams + await injectPullDiff(owner, repo, pull, window.document) return } - const { owner, repo, pull } = params - console.log('Found pull request diff URL', owner, repo, pull) - try { - await injectPullDiff(owner, repo, pull, window.document) - } catch (e) { - console.error(e) + + const commitParams = getGithubCommitUrlParams(url) + if (commitParams) { + const { owner, repo, sha } = commitParams + await injectCommitDiff(owner, repo, sha, window.document) + return } }) diff --git a/src/chrome/types.ts b/src/chrome/types.ts index f6d11a1..fa54a4f 100644 --- a/src/chrome/types.ts +++ b/src/chrome/types.ts @@ -11,6 +11,7 @@ export type DiffEntry = components['schemas']['diff-entry'] export type ContentFile = components['schemas']['content-file'] export type User = components['schemas']['simple-user'] export type Pull = components['schemas']['pull-request'] +export type Commit = components['schemas']['commit'] // chrome extension @@ -27,6 +28,7 @@ export enum MessageIds { GetKittycadUser = 'GetKittyCadUser', GetFileDiff = 'GetFileDiff', GetGithubPull = 'GetGithubPull', + GetGithubCommit = 'GetGithubCommit', } export type MessageGetGithubPullFilesData = { @@ -35,6 +37,12 @@ export type MessageGetGithubPullFilesData = { pull: number } +export type MessageGetGithubCommitData = { + owner: string + repo: string + sha: string +} + export type MessageGetFileDiff = { owner: string repo: string @@ -49,12 +57,17 @@ export type MessageSaveToken = { export type Message = { id: MessageIds - data?: MessageGetGithubPullFilesData | MessageSaveToken | MessageGetFileDiff + data?: + | MessageGetGithubPullFilesData + | MessageGetGithubCommitData + | MessageSaveToken + | MessageGetFileDiff } export type MessageResponse = | DiffEntry[] | Pull + | Commit | User | KittycadUser | MessageSaveToken diff --git a/src/chrome/web.test.ts b/src/chrome/web.test.ts index 9fbfcb8..3d5dccc 100644 --- a/src/chrome/web.test.ts +++ b/src/chrome/web.test.ts @@ -1,22 +1,50 @@ import { DiffEntry } from './types' import { getElementFilename, - getGithubUrlParams, - getInjectablePullElements, - getWebPullElements, + getGithubCommitUrlParams, + getGithubPullUrlParams, + mapInjectableDiffElements, + getSupportedWebDiffElements, } from './web' const githubPullHtmlSnippet = ` -
-
-
+
+ + +
+ +
+
+ Git LFS file not shown +
+
+
-
+
+
+
+ + seesaw.obj + +
+
Git LFS file not shown
@@ -30,6 +58,21 @@ const githubPullHtmlDocument = parser.parseFromString( ) const githubPullFilesSample: DiffEntry[] = [ + { + sha: 'c24ca35738a99e6bf834e0ee141db27c62fce499', + filename: 'samples/file_center_of_mass/output.json', + status: 'modified', + additions: 3, + deletions: 3, + changes: 6, + blob_url: + 'https://github.com/KittyCAD/litterbox/blob/11510a02d8294cac5943b8ebdc416170f5b738b5/samples%2Ffile_center_of_mass%2Foutput.json', + raw_url: + 'https://github.com/KittyCAD/litterbox/raw/11510a02d8294cac5943b8ebdc416170f5b738b5/samples%2Ffile_center_of_mass%2Foutput.json', + contents_url: + 'https://api.github.com/repos/KittyCAD/litterbox/contents/samples%2Ffile_center_of_mass%2Foutput.json?ref=11510a02d8294cac5943b8ebdc416170f5b738b5', + patch: '@@ -1,8 +1,8 @@\n {\n "title": "output.json",\n "center_of_mass": [\n- -1.7249649e-08,\n- 2.96097,\n- -0.36378\n+ -0.12732863,\n+ 1.0363415,\n+ -9.5138624e-08\n ]\n }\n\\ No newline at end of file', + }, { sha: '2f35d962a711bea7a8bf57481b8717f7dedbe1c5', filename: 'samples/file_center_of_mass/output.obj', @@ -45,12 +88,27 @@ const githubPullFilesSample: DiffEntry[] = [ 'https://api.github.com/repos/KittyCAD/litterbox/contents/samples%2Ffile_center_of_mass%2Foutput.obj?ref=11510a02d8294cac5943b8ebdc416170f5b738b5', patch: '@@ -1,3 +1,3 @@\n version https://git-lfs.github.com/spec/v1\n-oid sha256:2a07f53add3eee88b80a0bbe0412cf91df3d3bd9d45934ce849e0440eff90ee1\n-size 62122\n+oid sha256:0c0eb961e7e0589d83693335408b90d3b8adae9f4054c3e396c6eedbc5ed16ec\n+size 62545', }, + { + sha: '2f35d962a711bea7a8bf57481b8717f7dedbe1c5', + filename: 'seesaw.obj', + status: 'modified', + additions: 2, + deletions: 2, + changes: 4, + blob_url: + 'https://github.com/KittyCAD/litterbox/blob/11510a02d8294cac5943b8ebdc416170f5b738b5/seesaw.obj', + raw_url: + 'https://github.com/KittyCAD/litterbox/raw/11510a02d8294cac5943b8ebdc416170f5b738b5/seesaw.obj', + contents_url: + 'https://api.github.com/repos/KittyCAD/litterbox/contents/seesaw.obj?ref=11510a02d8294cac5943b8ebdc416170f5b738b5', + patch: '@@ -1,3 +1,3 @@\n version https://git-lfs.github.com/spec/v1\n-oid sha256:2a07f53add3eee88b80a0bbe0412cf91df3d3bd9d45934ce849e0440eff90ee1\n-size 62122\n+oid sha256:0c0eb961e7e0589d83693335408b90d3b8adae9f4054c3e396c6eedbc5ed16ec\n+size 62545', + }, ] -describe('Function getGithubUrlParams', () => { +describe('Function getGithubPullUrlParams', () => { it('gets params out of a valid github pull request link', () => { - const pullUrl = 'https://github.com/KittyCAD/kittycad.ts/pull/67/files' - const params = getGithubUrlParams(pullUrl) + const url = 'https://github.com/KittyCAD/kittycad.ts/pull/67/files' + const params = getGithubPullUrlParams(url) expect(params).toBeDefined() const { owner, repo, pull } = params! expect(owner).toEqual('KittyCAD') @@ -59,31 +117,50 @@ describe('Function getGithubUrlParams', () => { }) it("doesn't match other URLs", () => { - expect(getGithubUrlParams('http://google.com')).toBeUndefined() + expect(getGithubPullUrlParams('http://google.com')).toBeUndefined() expect( - getGithubUrlParams('https://github.com/KittyCAD/litterbox') + getGithubPullUrlParams('https://github.com/KittyCAD/litterbox') + ).toBeUndefined() + }) +}) + +describe('Function getGithubCommitUrlParams', () => { + it('gets params out of a valid github commit link', () => { + const url = + 'https://github.com/KittyCAD/litterbox/commit/4ddf899550addf41d6bf1b790ce79e46501411b3' + const params = getGithubCommitUrlParams(url) + expect(params).toBeDefined() + const { owner, repo, sha } = params! + expect(owner).toEqual('KittyCAD') + expect(repo).toEqual('litterbox') + expect(sha).toEqual('4ddf899550addf41d6bf1b790ce79e46501411b3') + }) + + it("doesn't match other URLs", () => { + expect(getGithubPullUrlParams('http://google.com')).toBeUndefined() + expect( + getGithubPullUrlParams('https://github.com/KittyCAD/litterbox') ).toBeUndefined() }) }) it('finds web elements for supported files', () => { - const elements = getWebPullElements(githubPullHtmlDocument) - expect(elements).toHaveLength(1) + const elements = getSupportedWebDiffElements(githubPullHtmlDocument) + expect(elements).toHaveLength(2) }) it('finds the filename of a supported file element', () => { - const elements = getWebPullElements(githubPullHtmlDocument) + const elements = getSupportedWebDiffElements(githubPullHtmlDocument) const filename = getElementFilename(elements[0]) expect(filename).toEqual('samples/file_center_of_mass/output.obj') }) it('finds injectable elements from html and api results', () => { - const elements = getWebPullElements(githubPullHtmlDocument) - const injectableElements = getInjectablePullElements( - elements, + const injectableElements = mapInjectableDiffElements( + githubPullHtmlDocument, githubPullFilesSample ) - expect(injectableElements).toHaveLength(1) + expect(injectableElements).toHaveLength(2) const { element, file } = injectableElements[0] expect(element).toBeDefined() expect(file).toBeDefined() diff --git a/src/chrome/web.ts b/src/chrome/web.ts index 6088f97..521b89d 100644 --- a/src/chrome/web.ts +++ b/src/chrome/web.ts @@ -1,16 +1,15 @@ -import { supportedSrcFormats } from './diff' +import { isFilenameSupported, supportedSrcFormats } from './diff' import { DiffEntry } from './types' -export type GithubUrlParams = - | { - owner: string - repo: string - pull: number - } - | undefined +export type GithubPullUrlParams = { + owner: string + repo: string + pull: number +} -export function getGithubUrlParams(url: string): GithubUrlParams { - // TODO: support commit diff +export function getGithubPullUrlParams( + url: string +): GithubPullUrlParams | undefined { const pullRe = /https:\/\/github\.com\/([a-zA-Z0-9_.-]+)\/([a-zA-Z0-9_.-]+)\/pull\/(\d+)\/files/ const result = pullRe.exec(url) @@ -19,10 +18,32 @@ export function getGithubUrlParams(url: string): GithubUrlParams { } const [, owner, repo, pull] = result + console.log('Found a supported Github Pull Request URL:', owner, repo, pull) return { owner, repo, pull: parseInt(pull) } } -export function getWebPullElements(document: Document): HTMLElement[] { +export type GithubCommitUrlParams = { + owner: string + repo: string + sha: string +} + +export function getGithubCommitUrlParams( + url: string +): GithubCommitUrlParams | undefined { + const pullRe = + /https:\/\/github\.com\/([a-zA-Z0-9_.-]+)\/([a-zA-Z0-9_.-]+)\/commit\/(\w+)/ + const result = pullRe.exec(url) + if (!result) { + return undefined + } + + const [, owner, repo, sha] = result + console.log('Found a supported Github Commit URL:', owner, repo, sha) + return { owner, repo, sha } +} + +export function getSupportedWebDiffElements(document: Document): HTMLElement[] { const fileTypeSelectors = Array.from(supportedSrcFormats).map( t => `.file[data-file-type=".${t}"]` ) @@ -37,19 +58,25 @@ export function getElementFilename(element: HTMLElement) { return titleElement.getAttribute('title') } -export function getInjectablePullElements( - elements: HTMLElement[], +export function mapInjectableDiffElements( + document: Document, files: DiffEntry[] ) { - if (elements.length !== files.length) { + const supportedFiles = files.filter(f => isFilenameSupported(f.filename)) + console.log(`Found ${supportedFiles.length} supported files with the API`) + + const supportedElements = getSupportedWebDiffElements(document) + console.log(`Found ${supportedElements.length} elements in the web page`) + + if (supportedElements.length !== supportedFiles.length) { throw Error( - `elements and files have different length. Got ${elements.length} and ${files.length}` + `elements and files have different length. Got ${supportedElements.length} and ${files.length}` ) } - const injectableElements = [] - for (const [index, element] of elements.entries()) { - const apiFile = files[index] + const injectableElements: { element: HTMLElement; file: DiffEntry }[] = [] + for (const [index, element] of supportedElements.entries()) { + const apiFile = supportedFiles[index] const filename = getElementFilename(element) if (filename !== apiFile.filename) { throw Error(