From 7dd66439250d3b08b08487a963266d47a3c152be Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 19 Sep 2018 16:34:56 -0700 Subject: [PATCH 01/14] feat: wip gitlab support --- src/extension/manifest.spec.json | 11 +++-- src/extension/scripts/inject.tsx | 8 ++-- .../code_intelligence/code_intelligence.tsx | 6 ++- src/libs/gitlab/code_intelligence.ts | 41 +++++++++++++++++++ src/libs/gitlab/dom_functions.ts | 11 +++++ src/libs/gitlab/file_info.ts | 38 +++++++++++++++++ .../components/codeIntelStatusIndicator.scss | 29 ++++++++++++- 7 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 src/libs/gitlab/code_intelligence.ts create mode 100644 src/libs/gitlab/dom_functions.ts create mode 100644 src/libs/gitlab/file_info.ts diff --git a/src/extension/manifest.spec.json b/src/extension/manifest.spec.json index a9bb1169..e08d1aab 100644 --- a/src/extension/manifest.spec.json +++ b/src/extension/manifest.spec.json @@ -3,7 +3,8 @@ "version": "0.0.0", "name": "Sourcegraph", "manifest_version": 2, - "description": "Code intelligence for your code host and code reviews: hovers, documentation, definitions, and references in files, PRs, and diffs", + "description": + "Code intelligence for your code host and code reviews: hovers, documentation, definitions, and references in files, PRs, and diffs", "browser_action": { "default_title": "Sourcegraph", "default_icon": { @@ -43,6 +44,7 @@ { "matches": [ "https://github.com/*", + "https://gitlab.com/*", "https://sourcegraph.com/*", "https://localhost:3443/*", "http://localhost:32773/*" @@ -57,16 +59,18 @@ "activeTab", "contextMenus", "https://github.com/*", + "https://gitlab.com/*", "https://localhost:3443/*", "https://sourcegraph.com/*", "http://localhost:32773/*" ], - "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvyGmcOkw4cTnhO0bgl3fQLAdv1jZp8T1ZHYI+4d8FgwwVKLYWE+pAuJ/0LrI69ibed4Nnnw5YleB1xCpI+mzB56xfXWboKp6lljevKqWJ5TpJk/Vam3kSSoZwpmJRXnzmcM3qKpL6viUhTfwGmQO6WVTsN4YCx+KWXv97IyF6yDTgd6hwFsvCZY2n1ADgurrQkE6AcJ3kK4xZ14jaHllXEdFcqwh0+Am5qLcIJ1cNo5iFD35exXsjwdQbmpt8sEk5f95pK5FEEbJFmOTguu2fOZycqIoTgoDrbbhT5k9TUogZaN5Lup0Iwh0Cv60i4C1f7IdPrxHuaYmYCfoUezXnQIDAQAB" + "key": + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvyGmcOkw4cTnhO0bgl3fQLAdv1jZp8T1ZHYI+4d8FgwwVKLYWE+pAuJ/0LrI69ibed4Nnnw5YleB1xCpI+mzB56xfXWboKp6lljevKqWJ5TpJk/Vam3kSSoZwpmJRXnzmcM3qKpL6viUhTfwGmQO6WVTsN4YCx+KWXv97IyF6yDTgd6hwFsvCZY2n1ADgurrQkE6AcJ3kK4xZ14jaHllXEdFcqwh0+Am5qLcIJ1cNo5iFD35exXsjwdQbmpt8sEk5f95pK5FEEbJFmOTguu2fOZycqIoTgoDrbbhT5k9TUogZaN5Lup0Iwh0Cv60i4C1f7IdPrxHuaYmYCfoUezXnQIDAQAB" }, "prod": { "content_scripts": [ { - "matches": ["https://github.com/*", "https://sourcegraph.com/*"], + "matches": ["https://github.com/*", "https://gitlab.com/*", "https://sourcegraph.com/*"], "run_at": "document_end", "js": ["js/inject.bundle.js"] } @@ -77,6 +81,7 @@ "storage", "contextMenus", "https://github.com/*", + "https://gitlab.com/*", "https://sourcegraph.com/*" ] } diff --git a/src/extension/scripts/inject.tsx b/src/extension/scripts/inject.tsx index 371cc4d4..6f3ab8f4 100644 --- a/src/extension/scripts/inject.tsx +++ b/src/extension/scripts/inject.tsx @@ -19,6 +19,7 @@ import { featureFlags } from '../../shared/util/featureFlags' import { injectBitbucketServer } from '../../libs/bitbucket/inject' import { injectCodeIntelligence } from '../../libs/code_intelligence' import { injectGitHubApplication } from '../../libs/github/inject' +import { checkIsGitlab } from '../../libs/gitlab/code_intelligence' import { injectPhabricatorApplication } from '../../libs/phabricator/app' import { injectSourcegraphApp } from '../../libs/sourcegraph/inject' import { assertEnv } from '../envAssertion' @@ -56,6 +57,7 @@ function injectApplication(): void { const isBitbucket = document.querySelector('.bitbucket-header-logo') || document.querySelector('.aui-header-logo.aui-header-logo-bitbucket') + const isGitlab = checkIsGitlab() if (!isSourcegraphServer && !document.getElementById('ext-style-sheet')) { if (window.safari) { @@ -63,7 +65,7 @@ function injectApplication(): void { type: 'insertCSS', payload: { file: 'css/style.bundle.css', origin: window.location.origin }, }) - } else if (isPhabricator || isGitHub || isGitHubEnterprise || isBitbucket) { + } else if (isPhabricator || isGitHub || isGitHubEnterprise || isBitbucket || isGitlab) { const styleSheet = document.createElement('link') as HTMLLinkElement styleSheet.id = 'ext-style-sheet' styleSheet.rel = 'stylesheet' @@ -101,8 +103,8 @@ function injectApplication(): void { injectBitbucketServer() } - if (isGitHub || isPhabricator) { - if (await featureFlags.isEnabled('newInject')) { + if (isGitHub || isPhabricator || isGitlab) { + if (isGitlab || (await featureFlags.isEnabled('newInject'))) { const subscriptions = await injectCodeIntelligence() window.addEventListener('unload', () => subscriptions.unsubscribe()) } diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 88a0f503..cc8d2c0d 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -23,6 +23,7 @@ import { lspViaAPIXlang } from '../../shared/backend/lsp' import { ButtonProps, CodeViewToolbar } from '../../shared/components/CodeViewToolbar' import { eventLogger, sourcegraphUrl } from '../../shared/util/context' import { githubCodeHost } from '../github/code_intelligence' +import { gitlabCodeHost } from '../gitlab/code_intelligence' import { phabricatorCodeHost } from '../phabricator/code_intelligence' import { findCodeViews } from './code_views' @@ -156,7 +157,8 @@ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } { overlayMount.classList.add(`hover-overlay-mount__${codeHost.name}`) document.body.appendChild(overlayMount) - const relativeElement = document.body + // const relativeElement = document.body + const relativeElement = document.querySelector('.layout-page')! as HTMLElement const fetchJumpURL = createJumpURLFetcher(lspViaAPIXlang.fetchDefinition, toPrettyBlobURL) @@ -308,7 +310,7 @@ async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise * incomplete setup requests. */ export async function injectCodeIntelligence(): Promise { - const codeHosts: CodeHost[] = [githubCodeHost, phabricatorCodeHost] + const codeHosts: CodeHost[] = [githubCodeHost, gitlabCodeHost, phabricatorCodeHost] return await injectCodeIntelligenceToCodeHosts(codeHosts) } diff --git a/src/libs/gitlab/code_intelligence.ts b/src/libs/gitlab/code_intelligence.ts new file mode 100644 index 00000000..962676d8 --- /dev/null +++ b/src/libs/gitlab/code_intelligence.ts @@ -0,0 +1,41 @@ +import { CodeHost, CodeView } from '../code_intelligence' +import { singleFileDOMFunctions } from './dom_functions' +import { resolveFileInfo } from './file_info' + +const toolbarButtonProps = { + className: 'btn btn-default btn-sm', + style: { marginRight: '5px', textDecoration: 'none', color: 'inherit' }, +} + +export function checkIsGitlab(): boolean { + return !!document.head.querySelector('meta[content="GitLab"]') +} + +const createToolbarMount = (codeView: HTMLElement) => { + const fileActions = codeView.querySelector('.file-actions') + if (!fileActions) { + throw new Error('Unable to find mount location') + } + + const mount = document.createElement('div') + mount.classList.add('btn-group') + mount.classList.add('sg-toolbar-mount') + + fileActions.insertAdjacentElement('afterbegin', mount) + + return mount +} + +const singleFileCodeView: CodeView = { + selector: '.file-holder', + dom: singleFileDOMFunctions, + getToolbarMount: createToolbarMount, + resolveFileInfo, + toolbarButtonProps, +} + +export const gitlabCodeHost: CodeHost = { + name: 'gitlab', + check: checkIsGitlab, + codeViews: [singleFileCodeView], +} diff --git a/src/libs/gitlab/dom_functions.ts b/src/libs/gitlab/dom_functions.ts new file mode 100644 index 00000000..5747ad45 --- /dev/null +++ b/src/libs/gitlab/dom_functions.ts @@ -0,0 +1,11 @@ +import { DOMFunctions } from '@sourcegraph/codeintellify' + +export const singleFileDOMFunctions: DOMFunctions = { + getCodeElementFromTarget: target => target.closest('span.line') as HTMLElement | null, + getLineNumberFromCodeElement: codeElement => { + const line = codeElement.id.replace(/^LC/, '') + + return parseInt(line, 10) + }, + getCodeElementFromLineNumber: (codeView, line) => codeView.querySelector(`#LC${line}`) as HTMLElement | null, +} diff --git a/src/libs/gitlab/file_info.ts b/src/libs/gitlab/file_info.ts new file mode 100644 index 00000000..18be9901 --- /dev/null +++ b/src/libs/gitlab/file_info.ts @@ -0,0 +1,38 @@ +import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' +import { Observable, of } from 'rxjs' +import { filter, map, switchMap } from 'rxjs/operators' + +import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' +import { FileInfo } from '../code_intelligence' + +function parseURL(): { repoPath: string; filePath: string; rev: string } { + const parts = window.location.href.split('/') + + const host = parts[2] + const owner = parts[3] + const repoName = parts[4] + const rev = parts[6] + const filePath = parts.slice(7).join('/') + + return { + repoPath: [host, owner, repoName].join('/'), + filePath, + rev, + } +} + +export const resolveFileInfo = (codeView: HTMLElement): Observable => + of(codeView).pipe( + map(() => { + const { repoPath, filePath, rev } = parseURL() + + return { repoPath, filePath, rev } + }), + filter(propertyIsDefined('filePath')), + switchMap(({ repoPath, rev, ...rest }) => + resolveRev({ repoPath, rev }).pipe( + retryWhenCloneInProgressError(), + map(commitID => ({ ...rest, repoPath, commitID, rev: rev || commitID })) + ) + ) + ) diff --git a/src/shared/components/codeIntelStatusIndicator.scss b/src/shared/components/codeIntelStatusIndicator.scss index 36f43561..79b4b04e 100644 --- a/src/shared/components/codeIntelStatusIndicator.scss +++ b/src/shared/components/codeIntelStatusIndicator.scss @@ -66,6 +66,33 @@ &__icon { width: 1rem !important; height: 1rem !important; - vertical-align: middle; + vertical-align: middle !important; + // top: unset !important; + } +} + +.sg-toolbar-mount { + .code-intel-status-indicator .btn { + line-height: 1rem; + } + + // & svg { + // position: unset !important; + // top: unset !important; + // } + + .btn { + padding: 0 10px; + font-size: 13px; + line-height: 28px; + display: inline-block; + float: none; + } + + .btn svg { + height: 15px !important; + width: 15px !important; + position: relative; + top: 2px; } } From a6d0ba857424e2b94c346c573a7db562cee4f041 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Fri, 21 Sep 2018 14:11:34 -0700 Subject: [PATCH 02/14] feat: wip --- .../code_intelligence/code_intelligence.tsx | 65 +++++++-- src/libs/gitlab/code_intelligence.ts | 40 +++++- src/libs/gitlab/dom_functions.ts | 23 +++- src/libs/gitlab/file_info.ts | 58 +++++--- src/libs/gitlab/scrape.ts | 125 ++++++++++++++++++ .../components/codeIntelStatusIndicator.scss | 41 +++--- 6 files changed, 293 insertions(+), 59 deletions(-) create mode 100644 src/libs/gitlab/scrape.ts diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index cc8d2c0d..f5ce9c55 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -65,6 +65,11 @@ export interface CodeViewResolver { resolveCodeView: (elem: HTMLElement) => CodeViewWithOutSelector } +interface OverlayPosition { + top: number + left: number +} + /** Information for adding code intelligence to code views on arbitrary code hosts. */ export interface CodeHost { /** @@ -72,6 +77,12 @@ export interface CodeHost { */ name: string + /** + * Checks to see if the current context the code is running in is within + * the given code host. + */ + check: () => Promise | boolean + /** * The list of types of code views to try to annotate. */ @@ -84,10 +95,11 @@ export interface CodeHost { codeViewResolver?: CodeViewResolver /** - * Checks to see if the current context the code is running in is within - * the given code host. + * Adjust the position of the hover overlay. Useful for fixed headers or other + * elements that throw off the position of the tooltip within the relative + * element. */ - check: () => Promise | boolean + adjustOverlayPosition?: (position: OverlayPosition) => OverlayPosition } export interface FileInfo { @@ -151,14 +163,21 @@ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } { const hoverOverlayElements = new Subject() const nextOverlayElement = (element: HTMLElement | null) => hoverOverlayElements.next(element) - const overlayMount = document.createElement('div') - overlayMount.style.height = '0px' - overlayMount.classList.add('hover-overlay-mount') - overlayMount.classList.add(`hover-overlay-mount__${codeHost.name}`) - document.body.appendChild(overlayMount) + const classNames = ['hover-overlay-mount', `hover-overlay-mount__${codeHost.name}`] + + const createMount = () => { + const overlayMount = document.createElement('div') + overlayMount.style.height = '0px' + for (const className of classNames) { + overlayMount.classList.add(className) + } + document.body.appendChild(overlayMount) + return overlayMount + } - // const relativeElement = document.body - const relativeElement = document.querySelector('.layout-page')! as HTMLElement + const overlayMount = document.querySelector(`.${classNames.join('.')}`) || createMount() + + const relativeElement = document.body const fetchJumpURL = createJumpURLFetcher(lspViaAPIXlang.fetchDefinition, toPrettyBlobURL) @@ -203,9 +222,10 @@ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } { containerComponentUpdates.next() } public render(): JSX.Element | null { - return this.state.hoverOverlayProps ? ( + const hoverOverlayProps = this.getHoverOverlayProps() + return hoverOverlayProps ? ( eventLogger.logCodeIntelligenceEvent() + private getHoverOverlayProps(): HoverState['hoverOverlayProps'] { + if (!this.state.hoverOverlayProps) { + return undefined + } + + let { overlayPosition, ...rest } = this.state.hoverOverlayProps + if (overlayPosition && codeHost.adjustOverlayPosition) { + overlayPosition = codeHost.adjustOverlayPosition(overlayPosition) + } + + return { + ...rest, + overlayPosition, + } + } } render(, overlayMount) @@ -257,7 +292,11 @@ function handleCodeHost(codeHost: CodeHost): Subscription { hoverifier.hoverify({ dom, positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), - resolveContext, + resolveContext: a => { + const b = resolveContext(a) + console.log(a, b) + return b + }, adjustPosition, }) ) diff --git a/src/libs/gitlab/code_intelligence.ts b/src/libs/gitlab/code_intelligence.ts index 962676d8..42dede63 100644 --- a/src/libs/gitlab/code_intelligence.ts +++ b/src/libs/gitlab/code_intelligence.ts @@ -1,6 +1,6 @@ -import { CodeHost, CodeView } from '../code_intelligence' -import { singleFileDOMFunctions } from './dom_functions' -import { resolveFileInfo } from './file_info' +import { CodeHost, CodeViewResolver, CodeViewWithOutSelector } from '../code_intelligence' +import { diffDOMFunctions, singleFileDOMFunctions } from './dom_functions' +import { resolveDiffFileInfo, resolveFileInfo } from './file_info' const toolbarButtonProps = { className: 'btn btn-default btn-sm', @@ -11,6 +11,15 @@ export function checkIsGitlab(): boolean { return !!document.head.querySelector('meta[content="GitLab"]') } +const adjustOverlayPosition: CodeHost['adjustOverlayPosition'] = ({ top, left }) => { + const header = document.querySelector('header') + + return { + top: header ? top + header.getBoundingClientRect().height : 0, + left, + } +} + const createToolbarMount = (codeView: HTMLElement) => { const fileActions = codeView.querySelector('.file-actions') if (!fileActions) { @@ -20,22 +29,41 @@ const createToolbarMount = (codeView: HTMLElement) => { const mount = document.createElement('div') mount.classList.add('btn-group') mount.classList.add('sg-toolbar-mount') + mount.classList.add('sg-toolbar-mount-gitlab') fileActions.insertAdjacentElement('afterbegin', mount) return mount } -const singleFileCodeView: CodeView = { - selector: '.file-holder', +const singleFileCodeView: CodeViewWithOutSelector = { dom: singleFileDOMFunctions, getToolbarMount: createToolbarMount, resolveFileInfo, toolbarButtonProps, } +const diffCodeView: CodeViewWithOutSelector = { + dom: diffDOMFunctions, + getToolbarMount: createToolbarMount, + resolveFileInfo: resolveDiffFileInfo, + toolbarButtonProps, +} + +const resolveCodeView = (codeView: HTMLElement): CodeViewWithOutSelector => { + const isDiff = codeView.classList.contains('diff-file') + + return isDiff ? diffCodeView : singleFileCodeView +} + +const codeViewResolver: CodeViewResolver = { + selector: '.file-holder', + resolveCodeView, +} + export const gitlabCodeHost: CodeHost = { name: 'gitlab', check: checkIsGitlab, - codeViews: [singleFileCodeView], + codeViewResolver, + adjustOverlayPosition, } diff --git a/src/libs/gitlab/dom_functions.ts b/src/libs/gitlab/dom_functions.ts index 5747ad45..cec67349 100644 --- a/src/libs/gitlab/dom_functions.ts +++ b/src/libs/gitlab/dom_functions.ts @@ -7,5 +7,26 @@ export const singleFileDOMFunctions: DOMFunctions = { return parseInt(line, 10) }, - getCodeElementFromLineNumber: (codeView, line) => codeView.querySelector(`#LC${line}`) as HTMLElement | null, + getCodeElementFromLineNumber: (codeView, line) => codeView.querySelector(`#LC${line}`), +} + +export const diffDOMFunctions: DOMFunctions = { + getCodeElementFromTarget: singleFileDOMFunctions.getCodeElementFromTarget, + getLineNumberFromCodeElement: singleFileDOMFunctions.getLineNumberFromCodeElement, + getCodeElementFromLineNumber: (codeView, line, part) => { + const lineNumberElement = codeView.querySelector( + `.${part === 'base' ? 'old_line' : 'new_line'}[data-linenumber="${line}"]` + ) + if (!lineNumberElement) { + return null + } + + const row = lineNumberElement.closest('tr') + if (!row) { + return null + } + + return row.querySelector('span.line') + }, + getDiffCodePart: codeElement => (codeElement.closest('td')!.classList.contains('old') ? 'base' : 'head'), } diff --git a/src/libs/gitlab/file_info.ts b/src/libs/gitlab/file_info.ts index 18be9901..43a69742 100644 --- a/src/libs/gitlab/file_info.ts +++ b/src/libs/gitlab/file_info.ts @@ -1,30 +1,18 @@ import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' -import { Observable, of } from 'rxjs' +import { Observable, of, zip } from 'rxjs' import { filter, map, switchMap } from 'rxjs/operators' import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' import { FileInfo } from '../code_intelligence' +import { getDiffPageInfo, getFilePageInfo, getFilePathFromCodeView } from './scrape' -function parseURL(): { repoPath: string; filePath: string; rev: string } { - const parts = window.location.href.split('/') - - const host = parts[2] - const owner = parts[3] - const repoName = parts[4] - const rev = parts[6] - const filePath = parts.slice(7).join('/') - - return { - repoPath: [host, owner, repoName].join('/'), - filePath, - rev, - } -} - +/** + * Resolves file information for a page with a single file, not including diffs with only one file. + */ export const resolveFileInfo = (codeView: HTMLElement): Observable => - of(codeView).pipe( + of(undefined).pipe( map(() => { - const { repoPath, filePath, rev } = parseURL() + const { repoPath, filePath, rev } = getFilePageInfo() return { repoPath, filePath, rev } }), @@ -36,3 +24,35 @@ export const resolveFileInfo = (codeView: HTMLElement): Observable => ) ) ) + +export const resolveDiffFileInfo = (codeView: HTMLElement): Observable => + of(undefined).pipe( + map(getDiffPageInfo), + switchMap(({ repoPath, rev, baseRev, ...rest }) => { + const resolvingHeadRev = resolveRev({ repoPath, rev }).pipe(retryWhenCloneInProgressError()) + const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) + + return zip(resolvingHeadRev, resolvingBaseRev).pipe( + map(([commitID, baseCommitID]) => ({ + repoPath, + rev, + baseRev, + commitID, + baseCommitID, + ...rest, + })) + ) + }), + map(info => ({ + ...info, + filePath: getFilePathFromCodeView(codeView), + // TODO: Look for new file names as well + baseFilePath: getFilePathFromCodeView(codeView), + })), + map(info => ({ + ...info, + + headHasFileContents: true, + baseHasFileContents: true, + })) + ) diff --git a/src/libs/gitlab/scrape.ts b/src/libs/gitlab/scrape.ts new file mode 100644 index 00000000..f4a7b613 --- /dev/null +++ b/src/libs/gitlab/scrape.ts @@ -0,0 +1,125 @@ +import { last } from 'lodash' + +interface GitLabInfo { + repoPath: string +} + +export interface GitLabFileInfo extends GitLabInfo { + filePath: string + rev: string +} + +/** + * Gets information about the page. + */ +function getPageInfo(): GitLabInfo { + const host = window.location.hostname + + const parts = window.location.pathname.slice(1).split('/') + + const owner = parts[0] + const repoName = parts[1] + + return { + repoPath: [host, owner, repoName].join('/'), + } +} + +/** + * Gets information about a file view page. + */ +export function getFilePageInfo(): GitLabFileInfo { + const { repoPath } = getPageInfo() + + const parts = window.location.pathname.slice(1).split('/') + + const rev = parts[3] + const filePath = parts.slice(4).join('/') + + return { + repoPath, + filePath, + rev, + } +} + +export interface GitLabDiffInfo extends Pick { + baseRev: string +} + +const createErrorBuilder = (message: string) => (kind: string) => new Error(`${message} (${kind})`) +const buildDiffPageError = createErrorBuilder('Unable to resolve diff information') + +const getHeadRev = (mergeContainer: HTMLElement) => { + const sourceBranch = mergeContainer.querySelector('.js-source-branch') + if (!sourceBranch) { + throw buildDiffPageError('no-source-branch') + } + + // If we have the branch name, use that. + if (sourceBranch.querySelector('a') && sourceBranch.dataset.originalTitle) { + return sourceBranch.dataset.originalTitle + } + + const commitAnchor = document.querySelector('a.commit-sha.js-mr-merged-commit-sha') + if (!commitAnchor) { + throw buildDiffPageError('no-head-rev-anchor') + } + + // Otherwise get the merged commit ID. + return last(commitAnchor.href!.split('/')) +} + +const getBaseRev = (mergeContainer: HTMLElement) => { + const targetBranch = mergeContainer.querySelector('a.js-target-branch') + console.log(targetBranch) + if (!targetBranch) { + throw buildDiffPageError('no-target-branch') + } + + return last(targetBranch.href!.split('/')) +} + +/** + * Scrapes the DOM for the repo path and revision information. + */ +export function getDiffPageInfo(): GitLabDiffInfo { + const mergeContainer = document.querySelector('.git-merge-container') + if (!mergeContainer) { + throw buildDiffPageError('no-merge-container') + } + + const rev = getHeadRev(mergeContainer) + if (!rev) { + throw buildDiffPageError('no-head-revision') + } + + const baseRev = getBaseRev(mergeContainer) + if (!baseRev) { + throw buildDiffPageError('no-base-revision') + } + + const { repoPath } = getPageInfo() + + return { + repoPath, + rev, + baseRev, + } +} + +const buildFileError = createErrorBuilder('Unable to file information') + +export function getFilePathFromCodeView(codeView: HTMLElement): string { + const filePathElem = codeView.querySelector('.file-title-name') + if (!filePathElem) { + throw buildFileError('no-file-title-element') + } + + const filePath = filePathElem.dataset.originalTitle + if (!filePath) { + throw buildFileError('no-file-title') + } + + return filePath +} diff --git a/src/shared/components/codeIntelStatusIndicator.scss b/src/shared/components/codeIntelStatusIndicator.scss index 79b4b04e..7d2ce84d 100644 --- a/src/shared/components/codeIntelStatusIndicator.scss +++ b/src/shared/components/codeIntelStatusIndicator.scss @@ -67,32 +67,33 @@ width: 1rem !important; height: 1rem !important; vertical-align: middle !important; - // top: unset !important; } } .sg-toolbar-mount { - .code-intel-status-indicator .btn { - line-height: 1rem; - } + &-gitlab { + .composite-container__header-action { + padding: 0 8px !important; + height: auto; + } - // & svg { - // position: unset !important; - // top: unset !important; - // } + & svg { + top: unset !important; + } - .btn { - padding: 0 10px; - font-size: 13px; - line-height: 28px; - display: inline-block; - float: none; - } + .btn { + padding: 0 10px; + font-size: 13px; + line-height: 28px; + display: inline-block; + float: none; + } - .btn svg { - height: 15px !important; - width: 15px !important; - position: relative; - top: 2px; + .btn svg { + height: 15px !important; + width: 15px !important; + position: relative; + top: 2px; + } } } From e00c370fa3c4fd934ad0f98c9d10600573df6bd0 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Fri, 21 Sep 2018 15:29:39 -0700 Subject: [PATCH 03/14] feat: wip --- src/libs/github/util.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/github/util.tsx b/src/libs/github/util.tsx index 5661ab42..88df43e4 100644 --- a/src/libs/github/util.tsx +++ b/src/libs/github/util.tsx @@ -202,6 +202,7 @@ export function getDiffResolvedRev(): DiffResolvedRevSpec | null { } } } + console.log(baseCommitID, headCommitID) } else { // Last-ditch: look for inline comment form input which has base/head on it. const baseInput = document.querySelector(`input[name="comparison_start_oid"]`) From de36adc2079203112113eaecf1107c95d9b8d7f1 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 25 Sep 2018 13:52:57 -0700 Subject: [PATCH 04/14] feat: diffs work reliably --- src/libs/code_intelligence/HoverOverlay.scss | 5 + .../code_intelligence/code_intelligence.tsx | 4 +- src/libs/gitlab/api.ts | 26 +++++ src/libs/gitlab/file_info.ts | 44 +++++--- src/libs/gitlab/scrape.ts | 105 ++++++++---------- 5 files changed, 104 insertions(+), 80 deletions(-) create mode 100644 src/libs/gitlab/api.ts diff --git a/src/libs/code_intelligence/HoverOverlay.scss b/src/libs/code_intelligence/HoverOverlay.scss index e8f23c08..acd6a3f1 100644 --- a/src/libs/code_intelligence/HoverOverlay.scss +++ b/src/libs/code_intelligence/HoverOverlay.scss @@ -16,3 +16,8 @@ } } } +.hover-overlay-mount__phabricator { + .hover-overlay { + z-index: 1000; + } +} diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index f5ce9c55..a83dd86c 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -284,7 +284,7 @@ function handleCodeHost(codeHost: CodeHost): Subscription { const resolveContext: ContextResolver = ({ part }) => ({ repoPath: part === 'base' ? info.baseRepoPath || info.repoPath : info.repoPath, commitID: part === 'base' ? info.baseCommitID! : info.commitID, - filePath: part === 'base' ? info.baseFilePath! : info.filePath, + filePath: part === 'base' ? info.baseFilePath || info.filePath : info.filePath, rev: part === 'base' ? info.baseRev || info.baseCommitID! : info.rev || info.commitID, }) @@ -294,7 +294,7 @@ function handleCodeHost(codeHost: CodeHost): Subscription { positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), resolveContext: a => { const b = resolveContext(a) - console.log(a, b) + console.log('ab', a, info, b) return b }, adjustPosition, diff --git a/src/libs/gitlab/api.ts b/src/libs/gitlab/api.ts new file mode 100644 index 00000000..fe99c847 --- /dev/null +++ b/src/libs/gitlab/api.ts @@ -0,0 +1,26 @@ +import { Observable } from 'rxjs' +import { ajax } from 'rxjs/ajax' +import { map } from 'rxjs/operators' + +import { memoizeObservable } from '../../shared/util/memoize' +import { GitLabDiffInfo } from './scrape' + +interface DiffRefs { + base_sha: string + head_sha: string + start_sha: string +} + +interface MRResponse { + diff_refs: DiffRefs +} + +type GetBaseCommitIDInput = Pick + +export const getBaseCommitID: (info: GetBaseCommitIDInput) => Observable = memoizeObservable( + ({ owner, repoName, mergeRequestID }: GetBaseCommitIDInput) => + ajax + .get(`${window.location.origin}/api/v4/projects/${owner}%2f${repoName}/merge_requests/${mergeRequestID}`) + .pipe(map(({ response }) => response as MRResponse), map(({ diff_refs: { base_sha } }) => base_sha)), + ({ mergeRequestID }) => mergeRequestID +) diff --git a/src/libs/gitlab/file_info.ts b/src/libs/gitlab/file_info.ts index 43a69742..c3b9ab86 100644 --- a/src/libs/gitlab/file_info.ts +++ b/src/libs/gitlab/file_info.ts @@ -4,7 +4,8 @@ import { filter, map, switchMap } from 'rxjs/operators' import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' import { FileInfo } from '../code_intelligence' -import { getDiffPageInfo, getFilePageInfo, getFilePathFromCodeView } from './scrape' +import { getBaseCommitID } from './api' +import { getDiffPageInfo, getFilePageInfo, getFilePathsFromCodeView, getHeadCommitIDFromCodeView } from './scrape' /** * Resolves file information for a page with a single file, not including diffs with only one file. @@ -28,31 +29,38 @@ export const resolveFileInfo = (codeView: HTMLElement): Observable => export const resolveDiffFileInfo = (codeView: HTMLElement): Observable => of(undefined).pipe( map(getDiffPageInfo), - switchMap(({ repoPath, rev, baseRev, ...rest }) => { - const resolvingHeadRev = resolveRev({ repoPath, rev }).pipe(retryWhenCloneInProgressError()) - const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) - - return zip(resolvingHeadRev, resolvingBaseRev).pipe( - map(([commitID, baseCommitID]) => ({ - repoPath, - rev, - baseRev, - commitID, - baseCommitID, - ...rest, - })) + switchMap(({ owner, repoName, mergeRequestID, ...rest }) => + getBaseCommitID({ owner, repoName, mergeRequestID }).pipe( + map(baseCommitID => ({ baseCommitID, baseRev: baseCommitID, ...rest })) ) + ), + map(info => { + const head = getHeadCommitIDFromCodeView(codeView) + + return { + ...info, + + rev: head, + commitID: head, + } }), map(info => ({ ...info, - filePath: getFilePathFromCodeView(codeView), - // TODO: Look for new file names as well - baseFilePath: getFilePathFromCodeView(codeView), + ...getFilePathsFromCodeView(codeView), })), map(info => ({ ...info, headHasFileContents: true, baseHasFileContents: true, - })) + })), + // Although we get the commit SHA's from elesewhere, we still need to + // use `resolveRev` otherwise we can't guarantee Sourcegraph has the + // rev cloned. + switchMap(({ repoPath, rev, baseRev, ...rest }) => { + const resolvingHeadRev = resolveRev({ repoPath, rev }).pipe(retryWhenCloneInProgressError()) + const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) + + return zip(resolvingHeadRev, resolvingBaseRev).pipe(map(() => ({ repoPath, rev, baseRev, ...rest }))) + }) ) diff --git a/src/libs/gitlab/scrape.ts b/src/libs/gitlab/scrape.ts index f4a7b613..a05af465 100644 --- a/src/libs/gitlab/scrape.ts +++ b/src/libs/gitlab/scrape.ts @@ -1,10 +1,16 @@ -import { last } from 'lodash' - interface GitLabInfo { repoPath: string + + owner: string + repoName: string + + /** + * The parts of the URL following the repo name. + */ + urlParts: string[] } -export interface GitLabFileInfo extends GitLabInfo { +export interface GitLabFileInfo extends Pick { filePath: string rev: string } @@ -21,7 +27,10 @@ function getPageInfo(): GitLabInfo { const repoName = parts[1] return { + owner, + repoName, repoPath: [host, owner, repoName].join('/'), + urlParts: parts.slice(2), } } @@ -43,83 +52,59 @@ export function getFilePageInfo(): GitLabFileInfo { } } -export interface GitLabDiffInfo extends Pick { - baseRev: string -} - const createErrorBuilder = (message: string) => (kind: string) => new Error(`${message} (${kind})`) -const buildDiffPageError = createErrorBuilder('Unable to resolve diff information') - -const getHeadRev = (mergeContainer: HTMLElement) => { - const sourceBranch = mergeContainer.querySelector('.js-source-branch') - if (!sourceBranch) { - throw buildDiffPageError('no-source-branch') - } - - // If we have the branch name, use that. - if (sourceBranch.querySelector('a') && sourceBranch.dataset.originalTitle) { - return sourceBranch.dataset.originalTitle - } - - const commitAnchor = document.querySelector('a.commit-sha.js-mr-merged-commit-sha') - if (!commitAnchor) { - throw buildDiffPageError('no-head-rev-anchor') - } - - // Otherwise get the merged commit ID. - return last(commitAnchor.href!.split('/')) -} - -const getBaseRev = (mergeContainer: HTMLElement) => { - const targetBranch = mergeContainer.querySelector('a.js-target-branch') - console.log(targetBranch) - if (!targetBranch) { - throw buildDiffPageError('no-target-branch') - } - return last(targetBranch.href!.split('/')) +export interface GitLabDiffInfo extends Pick, Pick { + mergeRequestID: string } /** * Scrapes the DOM for the repo path and revision information. */ export function getDiffPageInfo(): GitLabDiffInfo { - const mergeContainer = document.querySelector('.git-merge-container') - if (!mergeContainer) { - throw buildDiffPageError('no-merge-container') + const { repoPath, owner, repoName, urlParts } = getPageInfo() + + return { + repoPath, + owner, + repoName, + mergeRequestID: urlParts[1], } +} - const rev = getHeadRev(mergeContainer) - if (!rev) { - throw buildDiffPageError('no-head-revision') +const buildFileError = createErrorBuilder('Unable to file information') + +export function getFilePathsFromCodeView(codeView: HTMLElement): { filePath: string; baseFilePath?: string } { + const filePathElements = codeView.querySelectorAll('.file-title-name') + if (filePathElements.length === 0) { + throw buildFileError('no-file-title-element') } - const baseRev = getBaseRev(mergeContainer) - if (!baseRev) { - throw buildDiffPageError('no-base-revision') + const getFilePathFromElem = (elem: HTMLElement) => { + const filePath = elem.dataset.originalTitle + if (!filePath) { + throw buildFileError('no-file-title') + } + + return filePath } - const { repoPath } = getPageInfo() + const filePathDidChange = filePathElements.length > 1 return { - repoPath, - rev, - baseRev, + filePath: getFilePathFromElem(filePathElements.item(filePathDidChange ? 1 : 0)), + baseFilePath: filePathDidChange ? getFilePathFromElem(filePathElements.item(0)) : undefined, } } -const buildFileError = createErrorBuilder('Unable to file information') - -export function getFilePathFromCodeView(codeView: HTMLElement): string { - const filePathElem = codeView.querySelector('.file-title-name') - if (!filePathElem) { - throw buildFileError('no-file-title-element') +export function getHeadCommitIDFromCodeView(codeView: HTMLElement): string { + const commitSHA = codeView.querySelector('.file-actions .commit-sha') + if (!commitSHA) { + throw buildFileError('no-commit-sha') } - const filePath = filePathElem.dataset.originalTitle - if (!filePath) { - throw buildFileError('no-file-title') - } + const commitAnchor = commitSHA.closest('a')! as HTMLAnchorElement + const hrefParts = new URL(commitAnchor.href).pathname.slice(1).split('/') - return filePath + return hrefParts[3] } From fc999a948902bb394cecf1b0495d07958f709557 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 26 Sep 2018 09:24:37 -0700 Subject: [PATCH 05/14] feat: more reliable diff sha resolving --- src/libs/code_intelligence/HoverOverlay.scss | 2 +- src/libs/gitlab/api.ts | 28 ++++++++++++++++---- src/libs/gitlab/file_info.ts | 18 +++++++------ src/libs/gitlab/scrape.ts | 7 +++++ 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/libs/code_intelligence/HoverOverlay.scss b/src/libs/code_intelligence/HoverOverlay.scss index acd6a3f1..3cb0e400 100644 --- a/src/libs/code_intelligence/HoverOverlay.scss +++ b/src/libs/code_intelligence/HoverOverlay.scss @@ -16,7 +16,7 @@ } } } -.hover-overlay-mount__phabricator { +.hover-overlay-mount__gitlab { .hover-overlay { z-index: 1000; } diff --git a/src/libs/gitlab/api.ts b/src/libs/gitlab/api.ts index fe99c847..b0360f8a 100644 --- a/src/libs/gitlab/api.ts +++ b/src/libs/gitlab/api.ts @@ -15,12 +15,30 @@ interface MRResponse { diff_refs: DiffRefs } -type GetBaseCommitIDInput = Pick +interface DiffVersionsResponse { + base_commit_sha: string +} + +type GetBaseCommitIDInput = Pick export const getBaseCommitID: (info: GetBaseCommitIDInput) => Observable = memoizeObservable( - ({ owner, repoName, mergeRequestID }: GetBaseCommitIDInput) => - ajax - .get(`${window.location.origin}/api/v4/projects/${owner}%2f${repoName}/merge_requests/${mergeRequestID}`) - .pipe(map(({ response }) => response as MRResponse), map(({ diff_refs: { base_sha } }) => base_sha)), + ({ owner, repoName, mergeRequestID, diffID }: GetBaseCommitIDInput) => { + const mrURL = `${ + window.location.origin + }/api/v4/projects/${owner}%2f${repoName}/merge_requests/${mergeRequestID}` + + if (diffID) { + return ajax + .get(`${mrURL}/versions/${diffID}`) + .pipe( + map(({ response }) => response as DiffVersionsResponse), + map(({ base_commit_sha }) => base_commit_sha) + ) + } + + return ajax + .get(mrURL) + .pipe(map(({ response }) => response as MRResponse), map(({ diff_refs: { base_sha } }) => base_sha)) + }, ({ mergeRequestID }) => mergeRequestID ) diff --git a/src/libs/gitlab/file_info.ts b/src/libs/gitlab/file_info.ts index c3b9ab86..01f2a568 100644 --- a/src/libs/gitlab/file_info.ts +++ b/src/libs/gitlab/file_info.ts @@ -29,11 +29,13 @@ export const resolveFileInfo = (codeView: HTMLElement): Observable => export const resolveDiffFileInfo = (codeView: HTMLElement): Observable => of(undefined).pipe( map(getDiffPageInfo), - switchMap(({ owner, repoName, mergeRequestID, ...rest }) => - getBaseCommitID({ owner, repoName, mergeRequestID }).pipe( - map(baseCommitID => ({ baseCommitID, baseRev: baseCommitID, ...rest })) - ) - ), + switchMap(({ owner, repoName, mergeRequestID, diffID, baseCommitID, ...rest }) => { + const gettingBaseCommitID = baseCommitID + ? of(baseCommitID) + : getBaseCommitID({ owner, repoName, mergeRequestID, diffID }) + + return gettingBaseCommitID.pipe(map(baseCommitID => ({ baseCommitID, baseRev: baseCommitID, ...rest }))) + }), map(info => { const head = getHeadCommitIDFromCodeView(codeView) @@ -54,10 +56,10 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable headHasFileContents: true, baseHasFileContents: true, })), - // Although we get the commit SHA's from elesewhere, we still need to - // use `resolveRev` otherwise we can't guarantee Sourcegraph has the - // rev cloned. switchMap(({ repoPath, rev, baseRev, ...rest }) => { + // Although we get the commit SHA's from elesewhere, we still need to + // use `resolveRev` otherwise we can't guarantee Sourcegraph has the + // revision cloned. const resolvingHeadRev = resolveRev({ repoPath, rev }).pipe(retryWhenCloneInProgressError()) const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) diff --git a/src/libs/gitlab/scrape.ts b/src/libs/gitlab/scrape.ts index a05af465..7eef2b40 100644 --- a/src/libs/gitlab/scrape.ts +++ b/src/libs/gitlab/scrape.ts @@ -56,6 +56,9 @@ const createErrorBuilder = (message: string) => (kind: string) => new Error(`${m export interface GitLabDiffInfo extends Pick, Pick { mergeRequestID: string + + diffID?: string + baseCommitID?: string } /** @@ -64,11 +67,15 @@ export interface GitLabDiffInfo extends Pick, Pick Date: Wed, 26 Sep 2018 09:47:24 -0700 Subject: [PATCH 06/14] docs: add doc blocks --- src/libs/gitlab/api.ts | 33 +++++++++++++++++++++++---------- src/libs/gitlab/file_info.ts | 12 ++++++++++-- src/libs/gitlab/scrape.ts | 21 +++++++++++++++++++-- 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/libs/gitlab/api.ts b/src/libs/gitlab/api.ts index b0360f8a..fe64958a 100644 --- a/src/libs/gitlab/api.ts +++ b/src/libs/gitlab/api.ts @@ -5,40 +5,53 @@ import { map } from 'rxjs/operators' import { memoizeObservable } from '../../shared/util/memoize' import { GitLabDiffInfo } from './scrape' +/** + * Significant revisions for a merge request. + */ interface DiffRefs { base_sha: string head_sha: string start_sha: string } +/** + * Response from the GitLab API for fetching a merge request. Note that there + * is more information returned but we are not using it. + */ interface MRResponse { diff_refs: DiffRefs } +/** + * Response from the GitLab API for fetching a specific version(diff) of a merge + * request. Note that there is more information returned but we are not using it. + */ interface DiffVersionsResponse { base_commit_sha: string } type GetBaseCommitIDInput = Pick +const get = (url: string): Observable => ajax.get(url).pipe(map(({ response }) => response as T)) + +/** + * Get the base commit ID for a merge request. + */ export const getBaseCommitID: (info: GetBaseCommitIDInput) => Observable = memoizeObservable( ({ owner, repoName, mergeRequestID, diffID }: GetBaseCommitIDInput) => { const mrURL = `${ window.location.origin }/api/v4/projects/${owner}%2f${repoName}/merge_requests/${mergeRequestID}` + // If we have a `diffID`, retrieve the information for that individual diff. if (diffID) { - return ajax - .get(`${mrURL}/versions/${diffID}`) - .pipe( - map(({ response }) => response as DiffVersionsResponse), - map(({ base_commit_sha }) => base_commit_sha) - ) + return get(`${mrURL}/versions/${diffID}`).pipe( + map(({ base_commit_sha }) => base_commit_sha) + ) } - return ajax - .get(mrURL) - .pipe(map(({ response }) => response as MRResponse), map(({ diff_refs: { base_sha } }) => base_sha)) + // Otherwise, just get the overall base `commitID` for the merge request. + return get(mrURL).pipe(map(({ diff_refs: { base_sha } }) => base_sha)) }, - ({ mergeRequestID }) => mergeRequestID + ({ mergeRequestID, diffID }) => mergeRequestID + (diffID ? `/${diffID}` : '') ) diff --git a/src/libs/gitlab/file_info.ts b/src/libs/gitlab/file_info.ts index 01f2a568..687a4c0e 100644 --- a/src/libs/gitlab/file_info.ts +++ b/src/libs/gitlab/file_info.ts @@ -26,17 +26,24 @@ export const resolveFileInfo = (codeView: HTMLElement): Observable => ) ) +/** + * Gets `FileInfo` for a diff file. + */ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable => of(undefined).pipe( map(getDiffPageInfo), + // Resolve base commit ID. switchMap(({ owner, repoName, mergeRequestID, diffID, baseCommitID, ...rest }) => { const gettingBaseCommitID = baseCommitID - ? of(baseCommitID) - : getBaseCommitID({ owner, repoName, mergeRequestID, diffID }) + ? // Commit was found in URL. + of(baseCommitID) + : // Commit needs to be fetched from the API. + getBaseCommitID({ owner, repoName, mergeRequestID, diffID }) return gettingBaseCommitID.pipe(map(baseCommitID => ({ baseCommitID, baseRev: baseCommitID, ...rest }))) }), map(info => { + // Head commit is found in the "View file @ ..." button in the code view. const head = getHeadCommitIDFromCodeView(codeView) return { @@ -48,6 +55,7 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable }), map(info => ({ ...info, + // Find both head and base file path if the name has changed. ...getFilePathsFromCodeView(codeView), })), map(info => ({ diff --git a/src/libs/gitlab/scrape.ts b/src/libs/gitlab/scrape.ts index 7eef2b40..be478dbb 100644 --- a/src/libs/gitlab/scrape.ts +++ b/src/libs/gitlab/scrape.ts @@ -1,3 +1,8 @@ +import { FileInfo } from '../code_intelligence' + +/** + * General information that can be found on any GitLab page that we care about. (i.e. has code) + */ interface GitLabInfo { repoPath: string @@ -10,6 +15,9 @@ interface GitLabInfo { urlParts: string[] } +/** + * Information about single file pages. + */ export interface GitLabFileInfo extends Pick { filePath: string rev: string @@ -54,6 +62,9 @@ export function getFilePageInfo(): GitLabFileInfo { const createErrorBuilder = (message: string) => (kind: string) => new Error(`${message} (${kind})`) +/** + * Information specific to diff pages. + */ export interface GitLabDiffInfo extends Pick, Pick { mergeRequestID: string @@ -81,7 +92,10 @@ export function getDiffPageInfo(): GitLabDiffInfo { const buildFileError = createErrorBuilder('Unable to file information') -export function getFilePathsFromCodeView(codeView: HTMLElement): { filePath: string; baseFilePath?: string } { +/** + * Finds the file paths from the code view. If the name has changed, it'll return the base and head file paths. + */ +export function getFilePathsFromCodeView(codeView: HTMLElement): Pick { const filePathElements = codeView.querySelectorAll('.file-title-name') if (filePathElements.length === 0) { throw buildFileError('no-file-title-element') @@ -104,7 +118,10 @@ export function getFilePathsFromCodeView(codeView: HTMLElement): { filePath: str } } -export function getHeadCommitIDFromCodeView(codeView: HTMLElement): string { +/** + * Gets the head commit ID from the "View file @ ..." link on the code view. + */ +export function getHeadCommitIDFromCodeView(codeView: HTMLElement): FileInfo['commitID'] { const commitSHA = codeView.querySelector('.file-actions .commit-sha') if (!commitSHA) { throw buildFileError('no-commit-sha') From adedfa6216e7ee5bfb725579985aa4b32af8ee6b Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 26 Sep 2018 09:58:08 -0700 Subject: [PATCH 07/14] chore: rm console.log --- src/libs/code_intelligence/code_intelligence.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index a83dd86c..0215fdb3 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -292,11 +292,7 @@ function handleCodeHost(codeHost: CodeHost): Subscription { hoverifier.hoverify({ dom, positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), - resolveContext: a => { - const b = resolveContext(a) - console.log('ab', a, info, b) - return b - }, + resolveContext, adjustPosition, }) ) From b4804def0123e54807490b13078bf603e673441b Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 26 Sep 2018 11:14:00 -0700 Subject: [PATCH 08/14] fix: diff getCodeElementFromLineNumber --- src/libs/gitlab/dom_functions.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libs/gitlab/dom_functions.ts b/src/libs/gitlab/dom_functions.ts index cec67349..465ab2b4 100644 --- a/src/libs/gitlab/dom_functions.ts +++ b/src/libs/gitlab/dom_functions.ts @@ -15,7 +15,7 @@ export const diffDOMFunctions: DOMFunctions = { getLineNumberFromCodeElement: singleFileDOMFunctions.getLineNumberFromCodeElement, getCodeElementFromLineNumber: (codeView, line, part) => { const lineNumberElement = codeView.querySelector( - `.${part === 'base' ? 'old_line' : 'new_line'}[data-linenumber="${line}"]` + `.${part === 'base' ? 'old_line' : 'new_line'} [data-linenumber="${line}"]` ) if (!lineNumberElement) { return null @@ -26,7 +26,14 @@ export const diffDOMFunctions: DOMFunctions = { return null } - return row.querySelector('span.line') + let selector = 'span.line' + + // Split diff + if (row.classList.contains('parallel')) { + selector = `.${part === 'base' ? 'left-side' : 'right-side'} ${selector}` + } + + return row.querySelector(selector) }, getDiffCodePart: codeElement => (codeElement.closest('td')!.classList.contains('old') ? 'base' : 'head'), } From db825754a2d8c1260cebb3d9caf73af2d71da56c Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 26 Sep 2018 14:21:24 -0700 Subject: [PATCH 09/14] feat: commit page --- src/libs/gitlab/api.ts | 22 ++++++++-- src/libs/gitlab/code_intelligence.ts | 24 +++++++++-- src/libs/gitlab/file_info.ts | 62 +++++++++++++++++++++++----- src/libs/gitlab/scrape.ts | 46 +++++++++++++++++++-- 4 files changed, 131 insertions(+), 23 deletions(-) diff --git a/src/libs/gitlab/api.ts b/src/libs/gitlab/api.ts index fe64958a..8aa6faed 100644 --- a/src/libs/gitlab/api.ts +++ b/src/libs/gitlab/api.ts @@ -1,3 +1,4 @@ +import { first } from 'lodash' import { Observable } from 'rxjs' import { ajax } from 'rxjs/ajax' import { map } from 'rxjs/operators' @@ -32,16 +33,17 @@ interface DiffVersionsResponse { type GetBaseCommitIDInput = Pick +const buildURL = (owner: string, repoName: string, path: string) => + `${window.location.origin}/api/v4/projects/${owner}%2f${repoName}${path}` + const get = (url: string): Observable => ajax.get(url).pipe(map(({ response }) => response as T)) /** * Get the base commit ID for a merge request. */ -export const getBaseCommitID: (info: GetBaseCommitIDInput) => Observable = memoizeObservable( +export const getBaseCommitIDForMergeRequest: (info: GetBaseCommitIDInput) => Observable = memoizeObservable( ({ owner, repoName, mergeRequestID, diffID }: GetBaseCommitIDInput) => { - const mrURL = `${ - window.location.origin - }/api/v4/projects/${owner}%2f${repoName}/merge_requests/${mergeRequestID}` + const mrURL = buildURL(owner, repoName, `/merge_requests/${mergeRequestID}`) // If we have a `diffID`, retrieve the information for that individual diff. if (diffID) { @@ -55,3 +57,15 @@ export const getBaseCommitID: (info: GetBaseCommitIDInput) => Observable }, ({ mergeRequestID, diffID }) => mergeRequestID + (diffID ? `/${diffID}` : '') ) + +interface CommitResponse { + parent_ids: string[] +} + +export const getBaseCommitIDForCommit: ( + { owner, repoName, commitID }: Pick & { commitID: string } +) => Observable = memoizeObservable(({ owner, repoName, commitID }) => + get(buildURL(owner, repoName, `/repository/commits/${commitID}`)).pipe( + map(({ parent_ids }) => first(parent_ids)!) // ! because it'll always have a parent if we are looking at the commit page. + ) +) diff --git a/src/libs/gitlab/code_intelligence.ts b/src/libs/gitlab/code_intelligence.ts index 42dede63..b0c1c2ee 100644 --- a/src/libs/gitlab/code_intelligence.ts +++ b/src/libs/gitlab/code_intelligence.ts @@ -1,6 +1,7 @@ import { CodeHost, CodeViewResolver, CodeViewWithOutSelector } from '../code_intelligence' import { diffDOMFunctions, singleFileDOMFunctions } from './dom_functions' -import { resolveDiffFileInfo, resolveFileInfo } from './file_info' +import { resolveCommitFileInfo, resolveDiffFileInfo, resolveFileInfo } from './file_info' +import { getPageInfo, GitLabPageKind } from './scrape' const toolbarButtonProps = { className: 'btn btn-default btn-sm', @@ -43,17 +44,32 @@ const singleFileCodeView: CodeViewWithOutSelector = { toolbarButtonProps, } -const diffCodeView: CodeViewWithOutSelector = { +const mergeRequestCodeView: CodeViewWithOutSelector = { dom: diffDOMFunctions, getToolbarMount: createToolbarMount, resolveFileInfo: resolveDiffFileInfo, toolbarButtonProps, } +const commitCodeView: CodeViewWithOutSelector = { + dom: diffDOMFunctions, + getToolbarMount: createToolbarMount, + resolveFileInfo: resolveCommitFileInfo, + toolbarButtonProps, +} + const resolveCodeView = (codeView: HTMLElement): CodeViewWithOutSelector => { - const isDiff = codeView.classList.contains('diff-file') + const { pageKind } = getPageInfo() + + if (pageKind === GitLabPageKind.File) { + return singleFileCodeView + } + + if (pageKind === GitLabPageKind.MergeRequest) { + return mergeRequestCodeView + } - return isDiff ? diffCodeView : singleFileCodeView + return commitCodeView } const codeViewResolver: CodeViewResolver = { diff --git a/src/libs/gitlab/file_info.ts b/src/libs/gitlab/file_info.ts index 687a4c0e..724d6a7b 100644 --- a/src/libs/gitlab/file_info.ts +++ b/src/libs/gitlab/file_info.ts @@ -4,8 +4,27 @@ import { filter, map, switchMap } from 'rxjs/operators' import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' import { FileInfo } from '../code_intelligence' -import { getBaseCommitID } from './api' -import { getDiffPageInfo, getFilePageInfo, getFilePathsFromCodeView, getHeadCommitIDFromCodeView } from './scrape' +import { getBaseCommitIDForCommit, getBaseCommitIDForMergeRequest } from './api' +import { + getCommitPageInfo, + getDiffPageInfo, + getFilePageInfo, + getFilePathsFromCodeView, + getHeadCommitIDFromCodeView, +} from './scrape' + +const ensureRevisionsAreCloned = (files: Observable): Observable => + files.pipe( + switchMap(({ repoPath, rev, baseRev, ...rest }) => { + // Although we get the commit SHA's from elesewhere, we still need to + // use `resolveRev` otherwise we can't guarantee Sourcegraph has the + // revision cloned. + const resolvingHeadRev = resolveRev({ repoPath, rev }).pipe(retryWhenCloneInProgressError()) + const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) + + return zip(resolvingHeadRev, resolvingBaseRev).pipe(map(() => ({ repoPath, rev, baseRev, ...rest }))) + }) + ) /** * Resolves file information for a page with a single file, not including diffs with only one file. @@ -38,7 +57,7 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable ? // Commit was found in URL. of(baseCommitID) : // Commit needs to be fetched from the API. - getBaseCommitID({ owner, repoName, mergeRequestID, diffID }) + getBaseCommitIDForMergeRequest({ owner, repoName, mergeRequestID, diffID }) return gettingBaseCommitID.pipe(map(baseCommitID => ({ baseCommitID, baseRev: baseCommitID, ...rest }))) }), @@ -61,16 +80,37 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable map(info => ({ ...info, + // https://github.com/sourcegraph/browser-extensions/issues/185 headHasFileContents: true, baseHasFileContents: true, })), - switchMap(({ repoPath, rev, baseRev, ...rest }) => { - // Although we get the commit SHA's from elesewhere, we still need to - // use `resolveRev` otherwise we can't guarantee Sourcegraph has the - // revision cloned. - const resolvingHeadRev = resolveRev({ repoPath, rev }).pipe(retryWhenCloneInProgressError()) - const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) + ensureRevisionsAreCloned + ) - return zip(resolvingHeadRev, resolvingBaseRev).pipe(map(() => ({ repoPath, rev, baseRev, ...rest }))) - }) +/** + * Resolves file information for commit pages. + */ +export const resolveCommitFileInfo = (codeView: HTMLElement): Observable => + of(undefined).pipe( + map(getCommitPageInfo), + // Resolve base commit ID. + switchMap(({ owner, repoName, commitID, ...rest }) => + getBaseCommitIDForCommit({ owner, repoName, commitID }).pipe( + map(baseCommitID => ({ owner, repoName, commitID, baseCommitID, ...rest })) + ) + ), + map(info => ({ ...info, rev: info.commitID, baseRev: info.baseCommitID })), + map(info => ({ + ...info, + // Find both head and base file path if the name has changed. + ...getFilePathsFromCodeView(codeView), + })), + map(info => ({ + ...info, + + // https://github.com/sourcegraph/browser-extensions/issues/185 + headHasFileContents: true, + baseHasFileContents: true, + })), + ensureRevisionsAreCloned ) diff --git a/src/libs/gitlab/scrape.ts b/src/libs/gitlab/scrape.ts index be478dbb..e504e644 100644 --- a/src/libs/gitlab/scrape.ts +++ b/src/libs/gitlab/scrape.ts @@ -1,14 +1,24 @@ +import { last } from 'lodash' + import { FileInfo } from '../code_intelligence' +export enum GitLabPageKind { + File, + Commit, + MergeRequest, +} + /** * General information that can be found on any GitLab page that we care about. (i.e. has code) */ -interface GitLabInfo { - repoPath: string +export interface GitLabInfo { + pageKind: GitLabPageKind owner: string repoName: string + repoPath: string + /** * The parts of the URL following the repo name. */ @@ -26,7 +36,7 @@ export interface GitLabFileInfo extends Pick { /** * Gets information about the page. */ -function getPageInfo(): GitLabInfo { +export function getPageInfo(): GitLabInfo { const host = window.location.hostname const parts = window.location.pathname.slice(1).split('/') @@ -34,11 +44,21 @@ function getPageInfo(): GitLabInfo { const owner = parts[0] const repoName = parts[1] + let pageKind: GitLabPageKind + if (window.location.pathname.match(new RegExp(`${owner}/${repoName}/commit`))) { + pageKind = GitLabPageKind.Commit + } else if (window.location.pathname.match(new RegExp(`${owner}/${repoName}/merge_requests`))) { + pageKind = GitLabPageKind.MergeRequest + } else { + pageKind = GitLabPageKind.File + } + return { owner, repoName, repoPath: [host, owner, repoName].join('/'), urlParts: parts.slice(2), + pageKind, } } @@ -102,7 +122,7 @@ export function getFilePathsFromCodeView(codeView: HTMLElement): Pick { - const filePath = elem.dataset.originalTitle + const filePath = elem.dataset.originalTitle || elem.dataset.title if (!filePath) { throw buildFileError('no-file-title') } @@ -132,3 +152,21 @@ export function getHeadCommitIDFromCodeView(codeView: HTMLElement): FileInfo['co return hrefParts[3] } + +interface GitLabCommitPageInfo extends Pick, Pick { + commitID: FileInfo['commitID'] +} + +/** + * Get the commit from the URL. + */ +export function getCommitPageInfo(): GitLabCommitPageInfo { + const { repoPath, owner, repoName } = getPageInfo() + + return { + repoPath, + owner, + repoName, + commitID: last(window.location.pathname.split('/'))!, + } +} From 02c4fab489126e5aad7a2d554e8fef5ed7676aff Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 26 Sep 2018 15:40:40 -0700 Subject: [PATCH 10/14] feat: search --- .../code_intelligence/code_intelligence.tsx | 10 +++ src/libs/code_intelligence/search.ts | 79 +++++++++++++++++++ src/libs/gitlab/code_intelligence.ts | 2 + src/libs/gitlab/search.ts | 29 +++++++ 4 files changed, 120 insertions(+) create mode 100644 src/libs/code_intelligence/search.ts create mode 100644 src/libs/gitlab/search.ts diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 0215fdb3..08bc3fe6 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -26,6 +26,7 @@ import { githubCodeHost } from '../github/code_intelligence' import { gitlabCodeHost } from '../gitlab/code_intelligence' import { phabricatorCodeHost } from '../phabricator/code_intelligence' import { findCodeViews } from './code_views' +import { initSearch, SearchFeature } from './search' /** * Defines a type of code view a given code host can have. It tells us how to @@ -100,6 +101,11 @@ export interface CodeHost { * element. */ adjustOverlayPosition?: (position: OverlayPosition) => OverlayPosition + + /** + * Implementation of the search feature for a code host. + */ + search?: SearchFeature } export interface FileInfo { @@ -267,6 +273,10 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { } function handleCodeHost(codeHost: CodeHost): Subscription { + if (codeHost.search) { + initSearch(codeHost.search) + } + const { hoverifier } = initCodeIntelligence(codeHost) const subscriptions = new Subscription() diff --git a/src/libs/code_intelligence/search.ts b/src/libs/code_intelligence/search.ts new file mode 100644 index 00000000..40840373 --- /dev/null +++ b/src/libs/code_intelligence/search.ts @@ -0,0 +1,79 @@ +import storage from '../../browser/storage' +import { resolveRev } from '../../shared/repo/backend' +import { getPlatformName, repoUrlCache, sourcegraphUrl } from '../../shared/util/context' + +export interface SearchPageInformation { + query: string + repoPath: string + rev?: string +} + +/** + * Interface containing information needed for the search feature. + */ +export interface SearchFeature { + /** + * Check that we're on the search page. + */ + checkIsSearchPage: () => boolean + /** + * Get information required for executing a search. + */ + getRepoInformation: () => SearchPageInformation +} + +function getSourcegraphURLProps({ + repoPath, + rev, + query, +}: SearchPageInformation): { url: string; repo: string; rev: string | undefined; query: string } | undefined { + if (repoPath) { + if (rev) { + return { + url: `search?q=${encodeURIComponent(query)}&sq=repo:%5E${encodeURIComponent( + repoPath.replace(/\./g, '\\.') + )}%24@${encodeURIComponent(rev)}&utm_source=${getPlatformName()}`, + repo: repoPath, + rev, + query: `${encodeURIComponent(query)} ${encodeURIComponent( + repoPath.replace(/\./g, '\\.') + )}%24@${encodeURIComponent(rev)}`, + } + } + + return { + url: `search?q=${encodeURIComponent(query)}&sq=repo:%5E${encodeURIComponent( + repoPath.replace(/\./g, '\\.') + )}%24&utm_source=${getPlatformName()}`, + repo: repoPath, + rev, + query: `repo:^${repoPath.replace(/\./g, '\\.')}$ ${query}`, + } + } +} + +export function initSearch({ getRepoInformation, checkIsSearchPage }: SearchFeature): void { + if (checkIsSearchPage()) { + storage.getSync(({ executeSearchEnabled }) => { + // GitHub search page pathname is //search + if (false && !executeSearchEnabled) { + return + } + + const { repoPath, rev, query } = getRepoInformation() + if (query) { + const linkProps = getSourcegraphURLProps({ repoPath, rev, query }) + + if (linkProps) { + // Ensure that we open the correct sourcegraph server url by checking which + // server instance can access the repository. + resolveRev({ repoPath: linkProps.repo }).subscribe(() => { + const baseUrl = repoUrlCache[linkProps.repo] || sourcegraphUrl + const url = `${baseUrl}/${linkProps.url}` + window.open(url, '_blank') + }) + } + } + }) + } +} diff --git a/src/libs/gitlab/code_intelligence.ts b/src/libs/gitlab/code_intelligence.ts index b0c1c2ee..35239f57 100644 --- a/src/libs/gitlab/code_intelligence.ts +++ b/src/libs/gitlab/code_intelligence.ts @@ -2,6 +2,7 @@ import { CodeHost, CodeViewResolver, CodeViewWithOutSelector } from '../code_int import { diffDOMFunctions, singleFileDOMFunctions } from './dom_functions' import { resolveCommitFileInfo, resolveDiffFileInfo, resolveFileInfo } from './file_info' import { getPageInfo, GitLabPageKind } from './scrape' +import { search } from './search' const toolbarButtonProps = { className: 'btn btn-default btn-sm', @@ -82,4 +83,5 @@ export const gitlabCodeHost: CodeHost = { check: checkIsGitlab, codeViewResolver, adjustOverlayPosition, + search, } diff --git a/src/libs/gitlab/search.ts b/src/libs/gitlab/search.ts new file mode 100644 index 00000000..24829d68 --- /dev/null +++ b/src/libs/gitlab/search.ts @@ -0,0 +1,29 @@ +import { SearchFeature } from '../code_intelligence/search' + +const getRepoInformation: SearchFeature['getRepoInformation'] = () => { + const project = document.querySelector('.js-search-project-dropdown .dropdown-toggle-text') + if (!project) { + throw new Error('Unable to find project dropdown (search)') + } + + const projectText = project.textContent || '' + const parts = projectText + .trim() + .split(/\s/) + .slice(1) + + const owner = parts[0] + const repoName = parts[2] + + return { + query: new URLSearchParams(window.location.search).get('search') || '', + repoPath: `${window.location.host}/${owner}/${repoName}`, + } +} + +export const search: SearchFeature = { + checkIsSearchPage: () => + window.location.pathname === '/search' && + new URLSearchParams(window.location.search).get('search_code') === 'true', + getRepoInformation, +} From b3db104a723accd49caa6fa7e5e4014674fb8a21 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 26 Sep 2018 16:26:46 -0700 Subject: [PATCH 11/14] chore: run prettier --- src/extension/manifest.spec.json | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/extension/manifest.spec.json b/src/extension/manifest.spec.json index e08d1aab..5aa330ad 100644 --- a/src/extension/manifest.spec.json +++ b/src/extension/manifest.spec.json @@ -3,8 +3,7 @@ "version": "0.0.0", "name": "Sourcegraph", "manifest_version": 2, - "description": - "Code intelligence for your code host and code reviews: hovers, documentation, definitions, and references in files, PRs, and diffs", + "description": "Code intelligence for your code host and code reviews: hovers, documentation, definitions, and references in files, PRs, and diffs", "browser_action": { "default_title": "Sourcegraph", "default_icon": { @@ -64,8 +63,7 @@ "https://sourcegraph.com/*", "http://localhost:32773/*" ], - "key": - "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvyGmcOkw4cTnhO0bgl3fQLAdv1jZp8T1ZHYI+4d8FgwwVKLYWE+pAuJ/0LrI69ibed4Nnnw5YleB1xCpI+mzB56xfXWboKp6lljevKqWJ5TpJk/Vam3kSSoZwpmJRXnzmcM3qKpL6viUhTfwGmQO6WVTsN4YCx+KWXv97IyF6yDTgd6hwFsvCZY2n1ADgurrQkE6AcJ3kK4xZ14jaHllXEdFcqwh0+Am5qLcIJ1cNo5iFD35exXsjwdQbmpt8sEk5f95pK5FEEbJFmOTguu2fOZycqIoTgoDrbbhT5k9TUogZaN5Lup0Iwh0Cv60i4C1f7IdPrxHuaYmYCfoUezXnQIDAQAB" + "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvyGmcOkw4cTnhO0bgl3fQLAdv1jZp8T1ZHYI+4d8FgwwVKLYWE+pAuJ/0LrI69ibed4Nnnw5YleB1xCpI+mzB56xfXWboKp6lljevKqWJ5TpJk/Vam3kSSoZwpmJRXnzmcM3qKpL6viUhTfwGmQO6WVTsN4YCx+KWXv97IyF6yDTgd6hwFsvCZY2n1ADgurrQkE6AcJ3kK4xZ14jaHllXEdFcqwh0+Am5qLcIJ1cNo5iFD35exXsjwdQbmpt8sEk5f95pK5FEEbJFmOTguu2fOZycqIoTgoDrbbhT5k9TUogZaN5Lup0Iwh0Cv60i4C1f7IdPrxHuaYmYCfoUezXnQIDAQAB" }, "prod": { "content_scripts": [ From 2d6aea250ba5b713bfa8bdae4f34c606a83cb60a Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Thu, 27 Sep 2018 13:12:11 -0700 Subject: [PATCH 12/14] fix: diff getLineNumberFromCodeElement and address review comments --- src/libs/code_intelligence/search.ts | 2 +- src/libs/gitlab/dom_functions.ts | 27 ++++++++++++++++++++++++--- src/libs/gitlab/scrape.ts | 4 ++-- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/libs/code_intelligence/search.ts b/src/libs/code_intelligence/search.ts index 40840373..30e3c5de 100644 --- a/src/libs/code_intelligence/search.ts +++ b/src/libs/code_intelligence/search.ts @@ -56,7 +56,7 @@ export function initSearch({ getRepoInformation, checkIsSearchPage }: SearchFeat if (checkIsSearchPage()) { storage.getSync(({ executeSearchEnabled }) => { // GitHub search page pathname is //search - if (false && !executeSearchEnabled) { + if (!executeSearchEnabled) { return } diff --git a/src/libs/gitlab/dom_functions.ts b/src/libs/gitlab/dom_functions.ts index 465ab2b4..8b713b08 100644 --- a/src/libs/gitlab/dom_functions.ts +++ b/src/libs/gitlab/dom_functions.ts @@ -4,7 +4,6 @@ export const singleFileDOMFunctions: DOMFunctions = { getCodeElementFromTarget: target => target.closest('span.line') as HTMLElement | null, getLineNumberFromCodeElement: codeElement => { const line = codeElement.id.replace(/^LC/, '') - return parseInt(line, 10) }, getCodeElementFromLineNumber: (codeView, line) => codeView.querySelector(`#LC${line}`), @@ -12,7 +11,18 @@ export const singleFileDOMFunctions: DOMFunctions = { export const diffDOMFunctions: DOMFunctions = { getCodeElementFromTarget: singleFileDOMFunctions.getCodeElementFromTarget, - getLineNumberFromCodeElement: singleFileDOMFunctions.getLineNumberFromCodeElement, + getLineNumberFromCodeElement: codeElement => { + let cell: HTMLElement | null = codeElement.closest('td') + while (cell && !cell.dataset.linenumber && cell.previousElementSibling) { + cell = cell.previousElementSibling as HTMLElement | null + } + + if (cell) { + return parseInt(cell.dataset.linenumber || '', 10) + } + + throw new Error('Unable to determine line number for diff code element') + }, getCodeElementFromLineNumber: (codeView, line, part) => { const lineNumberElement = codeView.querySelector( `.${part === 'base' ? 'old_line' : 'new_line'} [data-linenumber="${line}"]` @@ -35,5 +45,16 @@ export const diffDOMFunctions: DOMFunctions = { return row.querySelector(selector) }, - getDiffCodePart: codeElement => (codeElement.closest('td')!.classList.contains('old') ? 'base' : 'head'), + getDiffCodePart: codeElement => { + let selector = 'old' + + const row = codeElement.closest('td')! + + // Split diff + if (row.classList.contains('parallel')) { + selector = 'left-side' + } + + return row.classList.contains(selector) ? 'base' : 'head' + }, } diff --git a/src/libs/gitlab/scrape.ts b/src/libs/gitlab/scrape.ts index e504e644..23e253f3 100644 --- a/src/libs/gitlab/scrape.ts +++ b/src/libs/gitlab/scrape.ts @@ -45,9 +45,9 @@ export function getPageInfo(): GitLabInfo { const repoName = parts[1] let pageKind: GitLabPageKind - if (window.location.pathname.match(new RegExp(`${owner}/${repoName}/commit`))) { + if (window.location.pathname.includes(`${owner}/${repoName}/commit`)) { pageKind = GitLabPageKind.Commit - } else if (window.location.pathname.match(new RegExp(`${owner}/${repoName}/merge_requests`))) { + } else if (window.location.pathname.includes(`${owner}/${repoName}/merge_requests`)) { pageKind = GitLabPageKind.MergeRequest } else { pageKind = GitLabPageKind.File From 86f93c7d7455cc01d1028ff3046bc38b167b48e8 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Thu, 27 Sep 2018 13:14:44 -0700 Subject: [PATCH 13/14] chore: rm log --- src/libs/github/util.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/github/util.tsx b/src/libs/github/util.tsx index 88df43e4..5661ab42 100644 --- a/src/libs/github/util.tsx +++ b/src/libs/github/util.tsx @@ -202,7 +202,6 @@ export function getDiffResolvedRev(): DiffResolvedRevSpec | null { } } } - console.log(baseCommitID, headCommitID) } else { // Last-ditch: look for inline comment form input which has base/head on it. const baseInput = document.querySelector(`input[name="comparison_start_oid"]`) From 2f7374d5b7830705544da5ff5f7a6ca3f2780262 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Thu, 27 Sep 2018 13:16:24 -0700 Subject: [PATCH 14/14] refactor: MRResponse -> MergeRequestResponse --- src/libs/gitlab/api.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libs/gitlab/api.ts b/src/libs/gitlab/api.ts index 8aa6faed..f1717f8a 100644 --- a/src/libs/gitlab/api.ts +++ b/src/libs/gitlab/api.ts @@ -19,7 +19,7 @@ interface DiffRefs { * Response from the GitLab API for fetching a merge request. Note that there * is more information returned but we are not using it. */ -interface MRResponse { +interface MergeRequestResponse { diff_refs: DiffRefs } @@ -53,7 +53,7 @@ export const getBaseCommitIDForMergeRequest: (info: GetBaseCommitIDInput) => Obs } // Otherwise, just get the overall base `commitID` for the merge request. - return get(mrURL).pipe(map(({ diff_refs: { base_sha } }) => base_sha)) + return get(mrURL).pipe(map(({ diff_refs: { base_sha } }) => base_sha)) }, ({ mergeRequestID, diffID }) => mergeRequestID + (diffID ? `/${diffID}` : '') ) @@ -62,6 +62,9 @@ interface CommitResponse { parent_ids: string[] } +/** + * Get the base commit ID for a commit. + */ export const getBaseCommitIDForCommit: ( { owner, repoName, commitID }: Pick & { commitID: string } ) => Observable = memoizeObservable(({ owner, repoName, commitID }) =>