From 8b9d0cae97a7386a0b87470dc84d26372e2726b5 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 11 Sep 2018 11:46:48 -0700 Subject: [PATCH 01/20] feat: define GitHub code_intelligence functions and objects --- .../{inject.tsx => code_intelligence.tsx} | 100 ++++++++++++------ src/libs/code_intelligence/index.ts | 1 + src/libs/github/code_intelligence.ts | 34 ++++++ src/libs/github/file_info.ts | 68 ++++++++++++ src/libs/github/util.tsx | 42 ++++++++ src/libs/phabricator/app.tsx | 2 +- src/libs/phabricator/code_views.ts | 2 +- src/libs/phabricator/extension.tsx | 2 +- src/libs/phabricator/file_info.ts | 2 +- src/shared/components/CodeViewToolbar.tsx | 2 +- 10 files changed, 217 insertions(+), 38 deletions(-) rename src/libs/code_intelligence/{inject.tsx => code_intelligence.tsx} (84%) create mode 100644 src/libs/code_intelligence/index.ts create mode 100644 src/libs/github/code_intelligence.ts create mode 100644 src/libs/github/file_info.ts diff --git a/src/libs/code_intelligence/inject.tsx b/src/libs/code_intelligence/code_intelligence.tsx similarity index 84% rename from src/libs/code_intelligence/inject.tsx rename to src/libs/code_intelligence/code_intelligence.tsx index b01c6a1e..69b54e9a 100644 --- a/src/libs/code_intelligence/inject.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -15,7 +15,7 @@ import { HoverMerged } from '@sourcegraph/codeintellify/lib/types' import { toPrettyBlobURL } from '@sourcegraph/codeintellify/lib/url' import * as React from 'react' import { render } from 'react-dom' -import { Observable, of, Subject, Subscription } from 'rxjs' +import { merge, Observable, of, Subject, Subscription } from 'rxjs' import { filter, map, withLatestFrom } from 'rxjs/operators' import { createJumpURLFetcher } from '../../shared/backend/lsp' @@ -23,6 +23,40 @@ import { lspViaAPIXlang } from '../../shared/backend/lsp' import { ButtonProps, CodeViewToolbar } from '../../shared/components/CodeViewToolbar' import { eventLogger, sourcegraphUrl } from '../../shared/util/context' +/** + * Defines a type of code view a given code host can have. It tells us how to + * look for the code view and how to do certain things when we find it. + */ +export interface CodeView { + /** A selector used by `document.querySelectorAll` to find the code view. */ + selector: string + /** The DOMFunctions for the code view. */ + dom: DOMFunctions + /** Finds or creates a DOM element where we should inject the `CodeViewToolbar`. */ + getToolbarMount?: (codeView: HTMLElement, part?: DiffPart) => HTMLElement + /** + * Resolves the file info for a given code view. It returns an observable + * because some code hosts need to resolve this asynchronously. The + * observable should only emit once. + */ + resolveFileInfo: (codeView: HTMLElement) => Observable + /** + * In some situations, we need to be able to adjust the position going into + * and coming out of codeintellify. For example, Phabricator converts tabs + * to spaces in it's DOM. + */ + adjustPosition?: PositionAdjuster + /** Props for styling the buttons in the `CodeViewToolbar`. */ + toolbarButtonProps?: ButtonProps +} + +export type CodeViewWithOutSelector = Pick> + +export interface CodeViewResolver { + selector: string + resolveCodeView: (elem: HTMLElement) => CodeViewWithOutSelector +} + /** Information for adding code intelligence to code views on arbitrary code hosts. */ export interface CodeHost { /** @@ -32,7 +66,13 @@ export interface CodeHost { /** * The list of types of code views to try to annotate. */ - codeViews: CodeView[] + codeViews?: CodeView[] + + /** + * Resolve `CodeView`s from the DOM. This is useful when each code view type + * doesn't have a distinct selector for + */ + codeViewResolver?: CodeViewResolver } export interface FileInfo { @@ -78,31 +118,6 @@ export interface FileInfo { baseHasFileContents?: boolean } -/** - * Defines a type of code view a given code host can have. It tells us how to - * look for the code view and how to do certain things when we find it. - */ -export interface CodeView { - /** A selector used by `document.querySelectorAll` to find the code view. */ - selector: string - /** The DOMFunctions for the code view. */ - dom: DOMFunctions - /** Finds or creates a DOM element where we should inject the `CodeViewToolbar`. */ - getToolbarMount?: (codeView: HTMLElement, part?: DiffPart) => HTMLElement - /** Resolves the file info for a given code view. It returns an observable - * because some code hosts need to resolve this asynchronously. The - * observable should only emit once. - */ - resolveFileInfo: (codeView: HTMLElement) => Observable - /** In some situations, we need to be able to adjust the position going into - * and coming out of codeintellify. For example, Phabricator converts tabs - * to spaces in it's DOM. - */ - adjustPosition?: PositionAdjuster - /** Props for styling the buttons in the `CodeViewToolbar`. */ - toolbarButtonProps?: ButtonProps -} - /** * Prepares the page for code intelligence. It creates the hoverifier, injects * and mounts the hover overlay and then returns the hoverifier. @@ -196,26 +211,45 @@ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } { * ResolvedCodeView attaches an actual code view DOM element that was found on * the page to the CodeView type being passed around by this file. */ -export interface ResolvedCodeView extends CodeView { +export interface ResolvedCodeView extends CodeViewWithOutSelector { /** The code view DOM element. */ codeView: HTMLElement } -function findCodeViews(codeViewInfos: CodeView[]): Observable { - return new Observable(observer => { - for (const info of codeViewInfos) { - const elements = document.querySelectorAll(info.selector) +function findCodeViews(codeHost: CodeHost): Observable { + const codeViewsFromList = new Observable(observer => { + if (!codeHost.codeViews) { + return + } + + for (const { selector, ...info } of codeHost.codeViews) { + const elements = document.querySelectorAll(selector) for (const codeView of elements) { observer.next({ ...info, codeView }) } } }) + + const codeViewsFromResolver = new Observable(observer => { + if (!codeHost.codeViewResolver) { + return + } + + const elements = document.querySelectorAll(codeHost.codeViewResolver.selector) + for (const elem of elements) { + const info = codeHost.codeViewResolver.resolveCodeView(elem) + + observer.next({ ...info, codeView: elem }) + } + }) + + return merge(codeViewsFromList, codeViewsFromResolver) } export function injectCodeIntelligence(codeHostInfo: CodeHost): Subscription { const { hoverifier } = initCodeIntelligence(codeHostInfo) - return findCodeViews(codeHostInfo.codeViews).subscribe( + return findCodeViews(codeHostInfo).subscribe( ({ codeView, dom, resolveFileInfo, adjustPosition, getToolbarMount, toolbarButtonProps }) => resolveFileInfo(codeView).subscribe(info => { const resolveContext: ContextResolver = ({ part }) => ({ diff --git a/src/libs/code_intelligence/index.ts b/src/libs/code_intelligence/index.ts new file mode 100644 index 00000000..52adc153 --- /dev/null +++ b/src/libs/code_intelligence/index.ts @@ -0,0 +1 @@ +export * from './code_intelligence' diff --git a/src/libs/github/code_intelligence.ts b/src/libs/github/code_intelligence.ts new file mode 100644 index 00000000..116fe6cd --- /dev/null +++ b/src/libs/github/code_intelligence.ts @@ -0,0 +1,34 @@ +import { CodeHost, CodeViewResolver, CodeViewWithOutSelector } from '../code_intelligence' +import { diffDomFunctions } from './dom_functions' +import { resolveDiffFileInfo } from './file_info' +import { createCodeViewToolbarMount, parseURL } from './util' + +const toolbarButtonProps = { + className: 'btn btn-sm tooltipped tooltipped-n', + style: { marginRight: '5px', textDecoration: 'none', color: 'inherit' }, +} + +const diffCodeView: CodeViewWithOutSelector = { + dom: diffDomFunctions, + getToolbarMount: createCodeViewToolbarMount, + resolveFileInfo: resolveDiffFileInfo, + toolbarButtonProps, +} + +const resolveCodeView = (elem: HTMLElement) => { + const files = document.getElementsByClassName('file') + const { filePath } = parseURL() + const isSingleCodeFile = files.length === 1 && filePath && document.getElementsByClassName('diff-view').length === 0 + + return isSingleCodeFile ? diffCodeView : diffCodeView +} + +const codeViewResolver: CodeViewResolver = { + selector: '.file', + resolveCodeView, +} + +export const githubCodeHost: CodeHost = { + name: 'github', + codeViewResolver, +} diff --git a/src/libs/github/file_info.ts b/src/libs/github/file_info.ts new file mode 100644 index 00000000..ae6076fe --- /dev/null +++ b/src/libs/github/file_info.ts @@ -0,0 +1,68 @@ +import { Observable, of, zip } from 'rxjs' +import { map, switchMap } from 'rxjs/operators' +import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' +import { FileInfo } from '../code_intelligence' +import { getDeltaFileName, getDiffResolvedRev, parseURL } from './util' + +export const resolveDiffFileInfo = (codeView: HTMLElement): Observable => + of(codeView).pipe( + map(codeView => { + const { repoPath } = parseURL() + + return { codeView, repoPath } + }), + map(({ codeView, ...rest }) => { + const { headFilePath, baseFilePath } = getDeltaFileName(codeView) + if (!headFilePath) { + throw new Error('cannot determine file path') + } + + return { ...rest, codeView, headFilePath, baseFilePath } + }), + map(({ codeView, ...rest }) => { + const diffResolvedRev = getDiffResolvedRev() + if (!diffResolvedRev) { + throw new Error('cannot determine delta info') + } + + return { + codeView, + headRev: diffResolvedRev.headCommitID, + baseRev: diffResolvedRev.baseCommitID, + ...rest, + } + }), + switchMap(({ repoPath, headRev, baseRev, ...rest }) => { + const resolvingHeadRev = resolveRev({ repoPath, rev: headRev }).pipe(retryWhenCloneInProgressError()) + const resolvingBaseRev = resolveRev({ repoPath, rev: baseRev }).pipe(retryWhenCloneInProgressError()) + + return zip(resolvingHeadRev, resolvingBaseRev).pipe( + map(([headCommitID, baseCommitID]) => ({ + repoPath, + headRev, + baseRev, + headCommitID, + baseCommitID, + ...rest, + })) + ) + }), + map(info => { + console.log('TODO: determine if files have contents', info) + + return { + repoPath: info.repoPath, + filePath: info.headFilePath, + commitID: info.headCommitID, + rev: info.headRev, + + baseRepoPath: info.repoPath, + baseFilePath: info.baseFilePath || info.headFilePath, + baseCommitID: info.baseCommitID, + baseRev: info.baseRev, + + headHasFileContents: true, + baseHasFileContents: true, + } + }) + ) diff --git a/src/libs/github/util.tsx b/src/libs/github/util.tsx index 338088f0..ee26a2ac 100644 --- a/src/libs/github/util.tsx +++ b/src/libs/github/util.tsx @@ -1,3 +1,5 @@ +import { DiffPart } from '@sourcegraph/codeintellify' + import { GitHubBlobUrl, GitHubMode, GitHubPullUrl, GitHubRepositoryUrl, GitHubURL } from '.' import { CodeCell, DiffRepoRev, DiffResolvedRevSpec, MaybeDiffSpec } from '../../shared/repo' import { parseHash } from '../../shared/util/url' @@ -57,6 +59,46 @@ export function createBlobAnnotatorMount(fileContainer: HTMLElement, isBase?: bo return mountEl } +/** + * Creates the mount element for the CodeViewToolbar. + */ +export function createCodeViewToolbarMount(fileContainer: HTMLElement, part?: DiffPart): HTMLElement { + const className = 'sourcegraph-app-annotator' + (part === 'base' ? '-base' : '') + const existingMount = fileContainer.querySelector('.' + className) as HTMLElement + if (existingMount) { + return existingMount + } + + const mountEl = document.createElement('div') + mountEl.style.display = 'inline-flex' + mountEl.style.verticalAlign = 'middle' + mountEl.style.alignItems = 'center' + mountEl.className = className + + const fileActions = fileContainer.querySelector('.file-actions') + if (!fileActions) { + throw new Error( + "File actions not found. Make sure you aren't trying to create " + + "a toolbar mount for a code snippet that shouldn't have one" + ) + } + + const buttonGroup = fileActions.querySelector('.BtnGroup') + if (buttonGroup && buttonGroup.parentNode && !fileContainer.querySelector('.show-file-notes')) { + // blob view + buttonGroup.parentNode.insertBefore(mountEl, buttonGroup) + } else { + // commit & pull request view + const note = fileContainer.querySelector('.show-file-notes') + if (!note || !note.parentNode) { + throw new Error('cannot find toolbar mount location') + } + note.parentNode.insertBefore(mountEl, note.nextSibling) + } + + return mountEl +} + export function isInlineCommentContainer(file: HTMLElement): boolean { return file.classList.contains('inline-review-comment') } diff --git a/src/libs/phabricator/app.tsx b/src/libs/phabricator/app.tsx index 380cc077..23f1a4c6 100644 --- a/src/libs/phabricator/app.tsx +++ b/src/libs/phabricator/app.tsx @@ -1,5 +1,5 @@ import { featureFlags } from '../../shared/util/featureFlags' -import { injectCodeIntelligence } from '../code_intelligence/inject' +import { injectCodeIntelligence } from '../code_intelligence' import { phabCodeViews } from './code_views' import { injectPhabricatorBlobAnnotators } from './inject_old' import { expanderListen, javelinPierce, metaClickOverride, setupPageLoadListener } from './util' diff --git a/src/libs/phabricator/code_views.ts b/src/libs/phabricator/code_views.ts index 28616312..90e968d7 100644 --- a/src/libs/phabricator/code_views.ts +++ b/src/libs/phabricator/code_views.ts @@ -2,7 +2,7 @@ import { AdjustmentDirection, DiffPart, PositionAdjuster } from '@sourcegraph/co import { map } from 'rxjs/operators' import { Position } from 'vscode-languageserver-types' import { fetchBlobContentLines } from '../../shared/repo/backend' -import { CodeView } from '../code_intelligence/inject' +import { CodeView } from '../code_intelligence' import { diffDomFunctions, diffusionDOMFns } from './dom_functions' import { resolveDiffFileInfo, resolveDiffusionFileInfo } from './file_info' import { convertSpacesToTabs, spacesToTabsAdjustment } from './index' diff --git a/src/libs/phabricator/extension.tsx b/src/libs/phabricator/extension.tsx index 813d690d..99dd74f9 100644 --- a/src/libs/phabricator/extension.tsx +++ b/src/libs/phabricator/extension.tsx @@ -2,7 +2,7 @@ import '../../config/polyfill' import { setSourcegraphUrl } from '../../shared/util/context' import { featureFlags } from '../../shared/util/featureFlags' -import { injectCodeIntelligence } from '../code_intelligence/inject' +import { injectCodeIntelligence } from '../code_intelligence' import { getPhabricatorCSS, getSourcegraphURLFromConduit } from './backend' import { phabCodeViews } from './code_views' import { injectPhabricatorBlobAnnotators } from './inject_old' diff --git a/src/libs/phabricator/file_info.ts b/src/libs/phabricator/file_info.ts index 36b82e35..33eeda34 100644 --- a/src/libs/phabricator/file_info.ts +++ b/src/libs/phabricator/file_info.ts @@ -2,7 +2,7 @@ import { from, Observable, zip } from 'rxjs' import { catchError, filter, map, switchMap } from 'rxjs/operators' import { DifferentialState, DiffusionState, PhabricatorMode } from '.' import { fetchBlobContentLines } from '../../shared/repo/backend' -import { FileInfo } from '../code_intelligence/inject' +import { FileInfo } from '../code_intelligence' import { resolveDiffRev } from './backend' import { getFilepathFromFile, getPhabricatorState } from './util' diff --git a/src/shared/components/CodeViewToolbar.tsx b/src/shared/components/CodeViewToolbar.tsx index 68576aed..5f702fcf 100644 --- a/src/shared/components/CodeViewToolbar.tsx +++ b/src/shared/components/CodeViewToolbar.tsx @@ -10,7 +10,7 @@ import { import * as React from 'react' import { Subscription } from 'rxjs' import { ContributableMenu } from 'sourcegraph/module/protocol' -import { FileInfo } from '../../libs/code_intelligence/inject' +import { FileInfo } from '../../libs/code_intelligence' import { SimpleProviderFns } from '../backend/lsp' import { fetchCurrentUser, fetchSite } from '../backend/server' import { CodeIntelStatusIndicator } from './CodeIntelStatusIndicator' From 8140dadd4c89fedfbc00ce14d8ab9e29de2522c7 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 11 Sep 2018 14:18:44 -0700 Subject: [PATCH 02/20] feat: use generic inject code intelligence function --- src/extension/scripts/inject.tsx | 10 + .../code_intelligence/code_intelligence.tsx | 30 +- src/libs/github/code_intelligence.ts | 11 + src/libs/github/inject.tsx | 357 +----------------- src/libs/phabricator/app.tsx | 3 - .../{code_views.ts => code_intelligence.ts} | 21 +- src/libs/phabricator/extension.tsx | 3 +- 7 files changed, 75 insertions(+), 360 deletions(-) rename src/libs/phabricator/{code_views.ts => code_intelligence.ts} (89%) diff --git a/src/extension/scripts/inject.tsx b/src/extension/scripts/inject.tsx index a3efbb5a..cebbe11d 100644 --- a/src/extension/scripts/inject.tsx +++ b/src/extension/scripts/inject.tsx @@ -17,6 +17,7 @@ import { 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 { injectPhabricatorApplication } from '../../libs/phabricator/app' import { injectSourcegraphApp } from '../../libs/sourcegraph/inject' @@ -96,6 +97,15 @@ function injectApplication(): void { setSourcegraphUrl(sourcegraphServerUrl) injectBitbucketServer() } + + if (isGitHub || isPhabricator) { + featureFlags.isEnabled('newTooltips').then(enabled => { + if (enabled) { + injectCodeIntelligence() + } + }) + } + setUseExtensions(items.useExtensions === undefined ? false : items.useExtensions) } diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 69b54e9a..730ba3a9 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -22,6 +22,8 @@ import { createJumpURLFetcher } from '../../shared/backend/lsp' 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 { phabricatorCodeHost } from '../phabricator/code_intelligence' /** * Defines a type of code view a given code host can have. It tells us how to @@ -63,6 +65,7 @@ export interface CodeHost { * The name of the code host. This will be added as a className to the overlay mount. */ name: string + /** * The list of types of code views to try to annotate. */ @@ -73,6 +76,12 @@ export interface CodeHost { * doesn't have a distinct selector for */ codeViewResolver?: CodeViewResolver + + /** + * Checks to see if the current context the code is running in is within + * the given code host. + */ + check: () => Promise | boolean } export interface FileInfo { @@ -95,7 +104,6 @@ export interface FileInfo { * The revision the code view is at. If a `baseRev` is provided, this value is treated as the head rev. */ rev?: string - /** * The repo bath for the BASE side of a diff. This is useful for Phabricator * staging areas since they are separate repos. @@ -246,10 +254,10 @@ function findCodeViews(codeHost: CodeHost): Observable { return merge(codeViewsFromList, codeViewsFromResolver) } -export function injectCodeIntelligence(codeHostInfo: CodeHost): Subscription { - const { hoverifier } = initCodeIntelligence(codeHostInfo) +function handleCodeHost(codeHost: CodeHost): Subscription { + const { hoverifier } = initCodeIntelligence(codeHost) - return findCodeViews(codeHostInfo).subscribe( + return findCodeViews(codeHost).subscribe( ({ codeView, dom, resolveFileInfo, adjustPosition, getToolbarMount, toolbarButtonProps }) => resolveFileInfo(codeView).subscribe(info => { const resolveContext: ContextResolver = ({ part }) => ({ @@ -288,3 +296,17 @@ export function injectCodeIntelligence(codeHostInfo: CodeHost): Subscription { }) ) } + +function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): void { + for (const codeHost of codeHosts) { + if (codeHost.check()) { + handleCodeHost(codeHost) + } + } +} + +export function injectCodeIntelligence(): void { + const codeHosts: CodeHost[] = [githubCodeHost, phabricatorCodeHost] + + injectCodeIntelligenceToCodeHosts(codeHosts) +} diff --git a/src/libs/github/code_intelligence.ts b/src/libs/github/code_intelligence.ts index 116fe6cd..765244dc 100644 --- a/src/libs/github/code_intelligence.ts +++ b/src/libs/github/code_intelligence.ts @@ -28,7 +28,18 @@ const codeViewResolver: CodeViewResolver = { resolveCodeView, } +function checkIsGithub(): boolean { + const href = window.location.href + + const isGithub = /^https?:\/\/(www.)?github.com/.test(href) + const ogSiteName = document.head.querySelector(`meta[property='og:site_name']`) as HTMLMetaElement + const isGitHubEnterprise = ogSiteName ? ogSiteName.content === 'GitHub Enterprise' : false + + return isGithub || isGitHubEnterprise +} + export const githubCodeHost: CodeHost = { name: 'github', codeViewResolver, + check: checkIsGithub, } diff --git a/src/libs/github/inject.tsx b/src/libs/github/inject.tsx index a85e424a..b77efcdf 100644 --- a/src/libs/github/inject.tsx +++ b/src/libs/github/inject.tsx @@ -16,14 +16,14 @@ import { } from '@sourcegraph/extensions-client-common/lib/client/controller' import { Controller } from '@sourcegraph/extensions-client-common/lib/controller' import { isErrorLike } from '@sourcegraph/extensions-client-common/lib/errors' -import { ConfigurationCascade } from '@sourcegraph/extensions-client-common/lib/settings' -import { ConfigurationSubject } from '@sourcegraph/extensions-client-common/lib/settings' import { ConfigurationCascadeOrError, ConfiguredSubject, Settings, } from '@sourcegraph/extensions-client-common/lib/settings' -import { identity } from 'lodash' +import { ConfigurationSubject } from '@sourcegraph/extensions-client-common/lib/settings' +import { ConfigurationCascade } from '@sourcegraph/extensions-client-common/lib/settings' + import mermaid from 'mermaid' import * as React from 'react' import { render, unmountComponentAtNode } from 'react-dom' @@ -31,10 +31,10 @@ import { combineLatest, forkJoin, from, of, Subject } from 'rxjs' import { filter, map, take, withLatestFrom } from 'rxjs/operators' import { ContributableMenu } from 'sourcegraph/module/protocol' import { Disposable } from 'vscode-languageserver' -import { findElementWithOffset, getTargetLineAndOffset, GitHubBlobUrl } from '.' +import { GitHubBlobUrl } from '.' import storage from '../../browser/storage' -import { createExtensionsContextController } from '../../shared/backend/extensions' import { applyDecoration, createMessageTransports } from '../../shared/backend/extensions' +import { createExtensionsContextController } from '../../shared/backend/extensions' import { createJumpURLFetcher, createLSPFromExtensions, @@ -44,16 +44,15 @@ import { toTextDocumentIdentifier, } from '../../shared/backend/lsp' import { Alerts } from '../../shared/components/Alerts' -import { BlobAnnotator } from '../../shared/components/BlobAnnotator' import { ConfigureSourcegraphButton } from '../../shared/components/ConfigureSourcegraphButton' import { ContextualSourcegraphButton } from '../../shared/components/ContextualSourcegraphButton' import { CodeViewToolbar } from '../../shared/components/LegacyCodeViewToolbar' import { ServerAuthButton } from '../../shared/components/ServerAuthButton' import { SymbolsDropdownContainer } from '../../shared/components/SymbolsDropdownContainer' import { WithResolvedRev } from '../../shared/components/WithResolvedRev' -import { AbsoluteRepoFile, CodeCell, DiffResolvedRevSpec } from '../../shared/repo' +import { AbsoluteRepoFile, DiffResolvedRevSpec } from '../../shared/repo' import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' -import { getTableDataCell, hideTooltip } from '../../shared/repo/tooltips' +import { hideTooltip } from '../../shared/repo/tooltips' import { RepoRevSidebar } from '../../shared/tree/RepoRevSidebar' import { eventLogger, @@ -69,7 +68,6 @@ import { blobDOMFunctions, diffDomFunctions, searchCodeSnippetDOMFunctions } fro import { initSearch } from './search' import { createBlobAnnotatorMount, - getCodeCells, getCodeCommentContainers, getDeltaFileName, getDiffRepoRev, @@ -77,12 +75,9 @@ import { getFileContainers, getGitHubState, getRepoCodeSearchContainers, - isDomSplitDiff, parseURL, } from './util' -const defaultFilterTarget = () => true - const buttonProps = { className: 'btn btn-sm tooltipped tooltipped-n', style: { marginRight: '5px', textDecoration: 'none', color: 'inherit' }, @@ -281,13 +276,8 @@ function inject(): void { featureFlags .isEnabled('newTooltips') .then(isEnabled => { - if (isEnabled) { + if (!isEnabled) { injectCodeIntelligence() - } else { - injectBlobAnnotatorsOld() - - injectCodeSnippetAnnotatorOld(getCodeCommentContainers(), '.border.rounded-1.my-2', false) - injectCodeSnippetAnnotatorOld(getRepoCodeSearchContainers(), '.d-inline-block', true) } }) .catch(err => console.error('could not get feature flag', err)) @@ -397,87 +387,6 @@ function injectFileTree(): void { specChanges.next({ repoPath, commitID: gitHubState.rev || '' }) } -const findTokenCell = (td: HTMLElement, target: HTMLElement) => { - let curr = target - while ( - curr.parentElement && - (curr.parentElement === td || curr.parentElement.classList.contains('blob-code-inner')) - ) { - curr = curr.parentElement - } - return curr -} - -/** - * injectCodeSnippetAnnotator annotates the given containers and adds a view file button. - * @param containers The blob containers that holds the code snippet to be annotated. - * @param selector The selector of the element to append a "View File" button. - */ -function injectCodeSnippetAnnotatorOld( - containers: HTMLCollectionOf, - selector: string, - isRepoSearch: boolean -): void { - for (const file of Array.from(containers)) { - const filePathContainer = file.querySelector(selector) - if (!filePathContainer) { - continue - } - const anchors = file.getElementsByTagName('a') - let gitHubState: GitHubBlobUrl | undefined - for (const anchor of Array.from(anchors)) { - const anchorState = getGitHubState(anchor.href) as GitHubBlobUrl - if (anchorState) { - gitHubState = anchorState - break - } - } - - if (!gitHubState || !gitHubState.owner || !gitHubState.repoName || !gitHubState.rev || !gitHubState.filePath) { - continue - } - const mountEl = document.createElement('div') - mountEl.style.display = 'none' - mountEl.className = 'sourcegraph-app-annotator' - filePathContainer.appendChild(mountEl) - - const getTableElement = () => file.querySelector('table') - - const getCodeCellsCb = () => { - const opt = { isDelta: false } - const table = getTableElement() - const cells: CodeCell[] = [] - if (!table) { - return cells - } - return getCodeCells(table, opt) - } - - render( - , - mountEl - ) - } -} - /** * injectCodeSnippetAnnotator annotates the given containers and adds a view file button. * @param containers The blob containers that holds the code snippet to be annotated. @@ -880,256 +789,6 @@ function injectBlobAnnotators( }) } -function injectBlobAnnotatorsOld(): void { - const { repoPath, isDelta, isPullRequest, rev, isCommit, filePath, position } = parseURL() - if (!filePath && !isDelta) { - return - } - - function addBlobAnnotator(file: HTMLElement): void { - const getTableElement = () => file.querySelector('table') - const diffLoader = file.querySelector('.js-diff-load-container') - if (diffLoader) { - const observer = new MutationObserver(() => { - const element = diffLoader.querySelector('.diff-table') - if (element) { - addBlobAnnotator(file) - observer.disconnect() - } - }) - observer.observe(diffLoader, { childList: true }) - } - - if (!isDelta) { - const mount = createBlobAnnotatorMount(file) - if (!mount) { - return - } - - const getCodeCellsCb = () => { - const opt = { isDelta: false } - const table = getTableElement() - const cells: CodeCell[] = [] - if (!table) { - return cells - } - return getCodeCells(table, opt) - } - - render( - , - mount - ) - return - } - - const { headFilePath, baseFilePath } = getDeltaFileName(file) - if (!headFilePath) { - console.error('cannot determine file path') - return - } - - const isSplitDiff = isDomSplitDiff() - let baseCommitID: string | undefined - let headCommitID: string | undefined - let baseRepoPath: string | undefined - const deltaRevs = getDiffResolvedRev() - if (!deltaRevs) { - console.error('cannot determine deltaRevs') - return - } - - baseCommitID = deltaRevs.baseCommitID - headCommitID = deltaRevs.headCommitID - - const deltaInfo = getDiffRepoRev() - if (!deltaInfo) { - console.error('cannot determine deltaInfo') - return - } - - baseRepoPath = deltaInfo.baseRepoPath - - const getCodeCellsDiff = (isBase: boolean) => () => { - const opt = { isDelta: true, isSplitDiff, isBase } - const table = getTableElement() - const cells: CodeCell[] = [] - if (!table) { - return cells - } - return getCodeCells(table, opt) - } - const getCodeCellsBase = getCodeCellsDiff(true) - const getCodeCellsHead = getCodeCellsDiff(false) - - const filterTarget = (isBase: boolean, isSplitDiff: boolean) => (target: HTMLElement) => { - const td = getTableDataCell(target) - if (!td) { - return false - } - - if (isSplitDiff) { - if (td.classList.contains('empty-cell')) { - return false - } - // Check the relative position of the element to determine if it is - // on the left or right. - const previousEl = td.previousElementSibling - const isLeft = previousEl === td.parentElement!.firstElementChild - if (isBase) { - return isLeft - } else { - return !isLeft - } - } - - if (td.classList.contains('blob-code-deletion') && !isBase) { - return false - } - if (td.classList.contains('blob-code-deletion') && isBase) { - return true - } - if (td.classList.contains('blob-code-addition') && isBase) { - return false - } - if (td.classList.contains('blob-code-addition') && !isBase) { - return true - } - if (isBase) { - return false - } - return true - } - - const getNodeToConvert = (td: HTMLTableDataCellElement) => { - if (!td.classList.contains('blob-code-inner')) { - return td.querySelector('.blob-code-inner') as HTMLElement - } - return td - } - - const mountHead = createBlobAnnotatorMount(file) - if (!mountHead) { - return - } - - render( - , - mountHead - ) - - const mountBase = createBlobAnnotatorMount(file, true) - if (!mountBase) { - return - } - - render( - , - mountBase - ) - } - - // Get first loaded files and annotate them. - const files = getFileContainers() - for (const file of Array.from(files)) { - addBlobAnnotator(file as HTMLElement) - } - const mutationObserver = new MutationObserver(mutations => { - for (const mutation of mutations) { - const nodes = Array.prototype.slice.call(mutation.addedNodes) - for (const node of nodes) { - if (node && node.classList && node.classList.contains('file') && node.classList.contains('js-file')) { - const intersectionObserver = new IntersectionObserver( - entries => { - for (const file of entries) { - // File is an IntersectionObserverEntry, which has `isIntersecting` as a prop, but TS - // complains that it does not exist. - if ((file as any).isIntersecting && !file.target.classList.contains('annotated')) { - file.target.classList.add('annotated') - addBlobAnnotator(file.target as HTMLElement) - } - } - }, - { - rootMargin: '200px', - threshold: 0, - } - ) - intersectionObserver.observe(node) - } - } - } - }) - const filebucket = document.getElementById('files') - if (!filebucket) { - return - } - - mutationObserver.observe(filebucket, { - childList: true, - subtree: true, - attributes: false, - characterData: false, - }) -} - /** * Appends an Open on Sourcegraph button to the GitHub DOM. * The button is only rendered on a repo homepage after the "find file" button. diff --git a/src/libs/phabricator/app.tsx b/src/libs/phabricator/app.tsx index 23f1a4c6..f2d37b3c 100644 --- a/src/libs/phabricator/app.tsx +++ b/src/libs/phabricator/app.tsx @@ -1,6 +1,4 @@ import { featureFlags } from '../../shared/util/featureFlags' -import { injectCodeIntelligence } from '../code_intelligence' -import { phabCodeViews } from './code_views' import { injectPhabricatorBlobAnnotators } from './inject_old' import { expanderListen, javelinPierce, metaClickOverride, setupPageLoadListener } from './util' @@ -27,7 +25,6 @@ function injectModules(): void { .isEnabled('newTooltips') .then(enabled => { if (enabled) { - injectCodeIntelligence({ codeViews: phabCodeViews, name: 'phabricator' }) return } diff --git a/src/libs/phabricator/code_views.ts b/src/libs/phabricator/code_intelligence.ts similarity index 89% rename from src/libs/phabricator/code_views.ts rename to src/libs/phabricator/code_intelligence.ts index 90e968d7..996a0ddf 100644 --- a/src/libs/phabricator/code_views.ts +++ b/src/libs/phabricator/code_intelligence.ts @@ -1,11 +1,12 @@ import { AdjustmentDirection, DiffPart, PositionAdjuster } from '@sourcegraph/codeintellify' import { map } from 'rxjs/operators' import { Position } from 'vscode-languageserver-types' +import { convertSpacesToTabs, spacesToTabsAdjustment } from '.' +import storage from '../../browser/storage' import { fetchBlobContentLines } from '../../shared/repo/backend' -import { CodeView } from '../code_intelligence' +import { CodeHost, CodeView } from '../code_intelligence' import { diffDomFunctions, diffusionDOMFns } from './dom_functions' import { resolveDiffFileInfo, resolveDiffusionFileInfo } from './file_info' -import { convertSpacesToTabs, spacesToTabsAdjustment } from './index' function createMount( findMountLocation: (file: HTMLElement, part?: DiffPart) => HTMLElement @@ -126,3 +127,19 @@ export const phabCodeViews: CodeView[] = [ toolbarButtonProps, }, ] + +function checkIsPhabricator(): Promise | boolean { + if (document.querySelector('.phabricator-wordmark')) { + return true + } + + return new Promise(resolve => + storage.getSync(items => resolve(!!items.enterpriseUrls.find(url => url === window.location.origin))) + ) +} + +export const phabricatorCodeHost: CodeHost = { + codeViews: phabCodeViews, + name: 'phabricator', + check: checkIsPhabricator, +} diff --git a/src/libs/phabricator/extension.tsx b/src/libs/phabricator/extension.tsx index 99dd74f9..3c16e32d 100644 --- a/src/libs/phabricator/extension.tsx +++ b/src/libs/phabricator/extension.tsx @@ -4,7 +4,6 @@ import { setSourcegraphUrl } from '../../shared/util/context' import { featureFlags } from '../../shared/util/featureFlags' import { injectCodeIntelligence } from '../code_intelligence' import { getPhabricatorCSS, getSourcegraphURLFromConduit } from './backend' -import { phabCodeViews } from './code_views' import { injectPhabricatorBlobAnnotators } from './inject_old' import { expanderListen, metaClickOverride, setupPageLoadListener } from './util' @@ -19,7 +18,7 @@ function injectModules(): void { .isEnabled('newTooltips') .then(enabled => { if (enabled) { - injectCodeIntelligence({ name: 'phabricator', codeViews: phabCodeViews }) + injectCodeIntelligence() return } From a41bfe4b9aa27ce14615f99b7a1b2d693771725e Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 11 Sep 2018 16:48:58 -0700 Subject: [PATCH 03/20] feat: annotate lazily loaded code views --- .../code_intelligence/code_intelligence.tsx | 89 +++++++++++++++++-- src/libs/github/util.tsx | 6 +- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 730ba3a9..3861b082 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -16,7 +16,7 @@ import { toPrettyBlobURL } from '@sourcegraph/codeintellify/lib/url' import * as React from 'react' import { render } from 'react-dom' import { merge, Observable, of, Subject, Subscription } from 'rxjs' -import { filter, map, withLatestFrom } from 'rxjs/operators' +import { filter, map, switchMap, withLatestFrom } from 'rxjs/operators' import { createJumpURLFetcher } from '../../shared/backend/lsp' import { lspViaAPIXlang } from '../../shared/backend/lsp' @@ -224,6 +224,14 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { codeView: HTMLElement } +/** + * Cast a Node to an HTMLElement if it has a classList. This should not be used + * if you need 100% confidence the Node is an HTMLElement. + */ +function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { + return !!(node as any).classList +} + function findCodeViews(codeHost: CodeHost): Observable { const codeViewsFromList = new Observable(observer => { if (!codeHost.codeViews) { @@ -251,7 +259,71 @@ function findCodeViews(codeHost: CodeHost): Observable { } }) - return merge(codeViewsFromList, codeViewsFromResolver) + const possibleLazyLoadedCodeViews = new Subject() + + const mutationObserver = new MutationObserver(mutations => { + for (const mutation of mutations) { + for (const node of mutation.addedNodes) { + if (!naiveCheckIsHTMLElement(node)) { + return + } + + possibleLazyLoadedCodeViews.next(node) + } + } + }) + + mutationObserver.observe(document.body, { + childList: true, + subtree: true, + attributes: false, + characterData: false, + }) + + const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazyLoadedCodeViews.pipe( + filter(() => !!codeHost.codeViews), + map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), + filter(propertyIsDefined('info')), + map(({ codeView, info }) => ({ ...info, codeView })) + ) + + const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazyLoadedCodeViews.pipe( + filter(() => !!codeHost.codeViewResolver), + map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), + filter(propertyIsDefined('info')), + map(({ codeView, info }) => ({ ...info, codeView })) + ) + + const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( + switchMap( + ({ codeView, ...rest }) => + new Observable(observer => { + const intersectionObserver = new IntersectionObserver( + entries => { + for (const entry of entries) { + // `entry` is an `IntersectionObserverEntry`, + // which has + // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) + // as a prop, but TS complains that it does not + // exist. + if ((entry as any).isIntersecting) { + observer.next({ codeView, ...rest }) + } + } + }, + { + rootMargin: '200px', + threshold: 0, + } + ) + intersectionObserver.observe(codeView) + }) + ) + ) + + return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( + filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) + ) } function handleCodeHost(codeHost: CodeHost): Subscription { @@ -274,6 +346,8 @@ function handleCodeHost(codeHost: CodeHost): Subscription { adjustPosition, }) + codeView.classList.add('sg-mounted') + if (!getToolbarMount) { return } @@ -299,9 +373,14 @@ function handleCodeHost(codeHost: CodeHost): Subscription { function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): void { for (const codeHost of codeHosts) { - if (codeHost.check()) { - handleCodeHost(codeHost) - } + const check = codeHost.check() + const checking = check instanceof Promise ? check : Promise.resolve(check) + + checking.then(isCodeHost => { + if (isCodeHost) { + handleCodeHost(codeHost) + } + }) } } diff --git a/src/libs/github/util.tsx b/src/libs/github/util.tsx index ee26a2ac..5661ab42 100644 --- a/src/libs/github/util.tsx +++ b/src/libs/github/util.tsx @@ -1,5 +1,3 @@ -import { DiffPart } from '@sourcegraph/codeintellify' - import { GitHubBlobUrl, GitHubMode, GitHubPullUrl, GitHubRepositoryUrl, GitHubURL } from '.' import { CodeCell, DiffRepoRev, DiffResolvedRevSpec, MaybeDiffSpec } from '../../shared/repo' import { parseHash } from '../../shared/util/url' @@ -62,8 +60,8 @@ export function createBlobAnnotatorMount(fileContainer: HTMLElement, isBase?: bo /** * Creates the mount element for the CodeViewToolbar. */ -export function createCodeViewToolbarMount(fileContainer: HTMLElement, part?: DiffPart): HTMLElement { - const className = 'sourcegraph-app-annotator' + (part === 'base' ? '-base' : '') +export function createCodeViewToolbarMount(fileContainer: HTMLElement): HTMLElement { + const className = 'sourcegraph-app-annotator' const existingMount = fileContainer.querySelector('.' + className) as HTMLElement if (existingMount) { return existingMount From b38e4ff6e6cb2785a83340bbb3a99e47e311f6ce Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 12 Sep 2018 14:51:59 -0700 Subject: [PATCH 04/20] refactor: findCodeElements is an operator --- .../code_intelligence/code_intelligence.tsx | 309 ++++++++++-------- 1 file changed, 176 insertions(+), 133 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 3861b082..e6291212 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -16,7 +16,7 @@ import { toPrettyBlobURL } from '@sourcegraph/codeintellify/lib/url' import * as React from 'react' import { render } from 'react-dom' import { merge, Observable, of, Subject, Subscription } from 'rxjs' -import { filter, map, switchMap, withLatestFrom } from 'rxjs/operators' +import { filter, map, mergeMap, switchMap, tap, withLatestFrom } from 'rxjs/operators' import { createJumpURLFetcher } from '../../shared/backend/lsp' import { lspViaAPIXlang } from '../../shared/backend/lsp' @@ -34,7 +34,11 @@ export interface CodeView { selector: string /** The DOMFunctions for the code view. */ dom: DOMFunctions - /** Finds or creates a DOM element where we should inject the `CodeViewToolbar`. */ + /** + * Finds or creates a DOM element where we should inject the + * `CodeViewToolbar`. This function is responsible for ensuring duplicate + * mounts aren't created. + */ getToolbarMount?: (codeView: HTMLElement, part?: DiffPart) => HTMLElement /** * Resolves the file info for a given code view. It returns an observable @@ -133,6 +137,7 @@ export interface FileInfo { * @param codeHost */ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } { + console.log('INIT code intel') /** Emits when the go to definition button was clicked */ const goToDefinitionClicks = new Subject() const nextGoToDefinitionClick = (event: MouseEvent) => goToDefinitionClicks.next(event) @@ -228,147 +233,181 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { * Cast a Node to an HTMLElement if it has a classList. This should not be used * if you need 100% confidence the Node is an HTMLElement. */ -function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { - return !!(node as any).classList -} - -function findCodeViews(codeHost: CodeHost): Observable { - const codeViewsFromList = new Observable(observer => { - if (!codeHost.codeViews) { - return - } - - for (const { selector, ...info } of codeHost.codeViews) { - const elements = document.querySelectorAll(selector) - for (const codeView of elements) { - observer.next({ ...info, codeView }) - } - } - }) - - const codeViewsFromResolver = new Observable(observer => { - if (!codeHost.codeViewResolver) { - return - } - - const elements = document.querySelectorAll(codeHost.codeViewResolver.selector) - for (const elem of elements) { - const info = codeHost.codeViewResolver.resolveCodeView(elem) - - observer.next({ ...info, codeView: elem }) - } - }) - - const possibleLazyLoadedCodeViews = new Subject() - - const mutationObserver = new MutationObserver(mutations => { - for (const mutation of mutations) { - for (const node of mutation.addedNodes) { - if (!naiveCheckIsHTMLElement(node)) { - return - } - - possibleLazyLoadedCodeViews.next(node) - } - } - }) - - mutationObserver.observe(document.body, { - childList: true, - subtree: true, - attributes: false, - characterData: false, - }) - - const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazyLoadedCodeViews.pipe( - filter(() => !!codeHost.codeViews), - map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), - filter(propertyIsDefined('info')), - map(({ codeView, info }) => ({ ...info, codeView })) - ) - - const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazyLoadedCodeViews.pipe( - filter(() => !!codeHost.codeViewResolver), - map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), - filter(propertyIsDefined('info')), - map(({ codeView, info }) => ({ ...info, codeView })) +// function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { +// return !!(node as any).classList +// } + +const findCodeViews = () => (codeHosts: Observable): Observable => { + const codeViewsFromList: Observable = codeHosts.pipe( + filter(propertyIsDefined('codeViews')), + switchMap(({ codeViews }) => + of(...codeViews).pipe( + map(({ selector, ...info }) => ({ + info, + matches: document.querySelectorAll(selector), + })) + ) + ), + switchMap(({ info, matches }) => + of(...matches).pipe( + map(codeView => ({ + ...info, + codeView, + })) + ) + ) ) - const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( - switchMap( - ({ codeView, ...rest }) => - new Observable(observer => { - const intersectionObserver = new IntersectionObserver( - entries => { - for (const entry of entries) { - // `entry` is an `IntersectionObserverEntry`, - // which has - // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) - // as a prop, but TS complains that it does not - // exist. - if ((entry as any).isIntersecting) { - observer.next({ codeView, ...rest }) - } - } - }, - { - rootMargin: '200px', - threshold: 0, - } - ) - intersectionObserver.observe(codeView) - }) + const codeViewsFromResolver: Observable = codeHosts.pipe( + filter(propertyIsDefined('codeViewResolver')), + map(({ codeViewResolver: { selector, resolveCodeView } }) => ({ + resolveCodeView, + matches: document.querySelectorAll(selector), + })), + switchMap(({ resolveCodeView, matches }) => + of(...matches).pipe( + map(codeView => ({ + ...resolveCodeView(codeView), + codeView, + })) + ) ) ) - return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( + // const codeViewsFromResolver = new Observable(observer => { + // if (!codeHost.codeViewResolver) { + // return + // } + // + // const elements = document.querySelectorAll(codeHost.codeViewResolver.selector) + // for (const elem of elements) { + // const info = codeHost.codeViewResolver.resolveCodeView(elem) + // + // observer.next({ ...info, codeView: elem }) + // } + // }) + // + // const possibleLazyLoadedCodeViews = new Subject() + // + // const mutationObserver = new MutationObserver(mutations => { + // for (const mutation of mutations) { + // console.log('mut', mutation) + // for (const node of mutation.addedNodes) { + // if (!naiveCheckIsHTMLElement(node)) { + // return + // } + // + // possibleLazyLoadedCodeViews.next(node) + // } + // } + // }) + // + // mutationObserver.observe(document.body, { + // // childList: true, + // subtree: true, + // attributes: false, + // characterData: false, + // }) + // + // const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazyLoadedCodeViews.pipe( + // filter(() => !!codeHost.codeViews), + // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), + // filter(propertyIsDefined('info')), + // map(({ codeView, info }) => ({ ...info, codeView })) + // ) + // + // const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazyLoadedCodeViews.pipe( + // filter(() => !!codeHost.codeViewResolver), + // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), + // filter(propertyIsDefined('info')), + // map(({ codeView, info }) => ({ ...info, codeView })) + // ) + // + // const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( + // switchMap( + // ({ codeView, ...rest }) => + // new Observable(observer => { + // const intersectionObserver = new IntersectionObserver( + // entries => { + // for (const entry of entries) { + // // `entry` is an `IntersectionObserverEntry`, + // // which has + // // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) + // // as a prop, but TS complains that it does not + // // exist. + // console.log('hello', entry) + // if ((entry as any).isIntersecting) { + // observer.next({ codeView, ...rest }) + // } + // } + // }, + // { + // rootMargin: '200px', + // threshold: 0, + // } + // ) + // intersectionObserver.observe(codeView) + // }) + // ) + // ) + + // return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( + // filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) + // ) + // + return merge(codeViewsFromList, codeViewsFromResolver).pipe( filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) ) } function handleCodeHost(codeHost: CodeHost): Subscription { const { hoverifier } = initCodeIntelligence(codeHost) + console.log('finding code views') + + return of(codeHost) + .pipe( + findCodeViews(), + mergeMap(({ codeView, resolveFileInfo, ...rest }) => + resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest }))) + ) + ) + .subscribe(({ codeView, info, dom, adjustPosition, getToolbarMount, toolbarButtonProps }) => { + 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, + rev: part === 'base' ? info.baseRev || info.baseCommitID! : info.rev || info.commitID, + }) - return findCodeViews(codeHost).subscribe( - ({ codeView, dom, resolveFileInfo, adjustPosition, getToolbarMount, toolbarButtonProps }) => - resolveFileInfo(codeView).subscribe(info => { - 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, - rev: part === 'base' ? info.baseRev || info.baseCommitID! : info.rev || info.commitID, - }) - - hoverifier.hoverify({ - dom, - positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), - resolveContext, - adjustPosition, - }) - - codeView.classList.add('sg-mounted') - - if (!getToolbarMount) { - return - } + hoverifier.hoverify({ + dom, + positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), + resolveContext, + adjustPosition, + }) - const mount = getToolbarMount(codeView) + codeView.classList.add('sg-mounted') - render( - , - mount - ) - }) - ) + } + simpleProviderFns={lspViaAPIXlang} + />, + mount + ) + }) } function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): void { @@ -376,11 +415,15 @@ function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): void { const check = codeHost.check() const checking = check instanceof Promise ? check : Promise.resolve(check) - checking.then(isCodeHost => { - if (isCodeHost) { - handleCodeHost(codeHost) - } - }) + checking + .then(isCodeHost => { + if (isCodeHost) { + handleCodeHost(codeHost) + } + }) + .catch(err => { + /* noop */ + }) } } From e9aab85f5cfe95adb19ed7e4dbfad132e66dda7e Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 12 Sep 2018 16:44:18 -0700 Subject: [PATCH 05/20] refactor: wip --- src/libs/code_intelligence/code_views.ts | 128 +++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 src/libs/code_intelligence/code_views.ts diff --git a/src/libs/code_intelligence/code_views.ts b/src/libs/code_intelligence/code_views.ts new file mode 100644 index 00000000..39e9541d --- /dev/null +++ b/src/libs/code_intelligence/code_views.ts @@ -0,0 +1,128 @@ +import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' +import { Observable, of } from 'rxjs' +import { filter, map, switchMap } from 'rxjs/operators' + +import { CodeHost, ResolvedCodeView } from './code_intelligence' + +export const findCodeViews = () => (codeHosts: Observable): Observable => { + const codeViewsFromList: Observable = codeHosts.pipe( + filter(propertyIsDefined('codeViews')), + switchMap(({ codeViews }) => + of(...codeViews).pipe( + map(({ selector, ...info }) => ({ + info, + matches: document.querySelectorAll(selector), + })) + ) + ), + switchMap(({ info, matches }) => + of(...matches).pipe( + map(codeView => ({ + ...info, + codeView, + })) + ) + ) + ) + + const codeViewsFromResolver: Observable = codeHosts.pipe( + filter(propertyIsDefined('codeViewResolver')), + map(({ codeViewResolver: { selector, resolveCodeView } }) => ({ + resolveCodeView, + matches: document.querySelectorAll(selector), + })), + switchMap(({ resolveCodeView, matches }) => + of(...matches).pipe( + map(codeView => ({ + ...resolveCodeView(codeView), + codeView, + })) + ) + ) + ) + + // const codeViewsFromResolver = new Observable(observer => { + // if (!codeHost.codeViewResolver) { + // return + // } + // + // const elements = document.querySelectorAll(codeHost.codeViewResolver.selector) + // for (const elem of elements) { + // const info = codeHost.codeViewResolver.resolveCodeView(elem) + // + // observer.next({ ...info, codeView: elem }) + // } + // }) + // + // const possibleLazyLoadedCodeViews = new Subject() + // + // const mutationObserver = new MutationObserver(mutations => { + // for (const mutation of mutations) { + // console.log('mut', mutation) + // for (const node of mutation.addedNodes) { + // if (!naiveCheckIsHTMLElement(node)) { + // return + // } + // + // possibleLazyLoadedCodeViews.next(node) + // } + // } + // }) + // + // mutationObserver.observe(document.body, { + // // childList: true, + // subtree: true, + // attributes: false, + // characterData: false, + // }) + // + // const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazyLoadedCodeViews.pipe( + // filter(() => !!codeHost.codeViews), + // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), + // filter(propertyIsDefined('info')), + // map(({ codeView, info }) => ({ ...info, codeView })) + // ) + // + // const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazyLoadedCodeViews.pipe( + // filter(() => !!codeHost.codeViewResolver), + // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), + // filter(propertyIsDefined('info')), + // map(({ codeView, info }) => ({ ...info, codeView })) + // ) + // + // const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( + // switchMap( + // ({ codeView, ...rest }) => + // new Observable(observer => { + // const intersectionObserver = new IntersectionObserver( + // entries => { + // for (const entry of entries) { + // // `entry` is an `IntersectionObserverEntry`, + // // which has + // // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) + // // as a prop, but TS complains that it does not + // // exist. + // console.log('hello', entry) + // if ((entry as any).isIntersecting) { + // observer.next({ codeView, ...rest }) + // } + // } + // }, + // { + // rootMargin: '200px', + // threshold: 0, + // } + // ) + // intersectionObserver.observe(codeView) + // }) + // ) + // ) + + // return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( + // filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) + // ) + // + return merge(codeViewsFromList, codeViewsFromResolver).pipe( + filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) + ) +} From d08e525f88c6adb13c43de06993657c3d66b0d1e Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Mon, 17 Sep 2018 15:59:09 -0700 Subject: [PATCH 06/20] feat: listens for new code views --- .../code_intelligence/code_intelligence.tsx | 140 +------------- src/libs/code_intelligence/code_views.ts | 173 +++++++++--------- 2 files changed, 88 insertions(+), 225 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index e6291212..38cb41bb 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -15,8 +15,8 @@ import { HoverMerged } from '@sourcegraph/codeintellify/lib/types' import { toPrettyBlobURL } from '@sourcegraph/codeintellify/lib/url' import * as React from 'react' import { render } from 'react-dom' -import { merge, Observable, of, Subject, Subscription } from 'rxjs' -import { filter, map, mergeMap, switchMap, tap, withLatestFrom } from 'rxjs/operators' +import { Observable, of, Subject, Subscription } from 'rxjs' +import { filter, map, mergeMap, withLatestFrom } from 'rxjs/operators' import { createJumpURLFetcher } from '../../shared/backend/lsp' import { lspViaAPIXlang } from '../../shared/backend/lsp' @@ -24,6 +24,7 @@ import { ButtonProps, CodeViewToolbar } from '../../shared/components/CodeViewTo import { eventLogger, sourcegraphUrl } from '../../shared/util/context' import { githubCodeHost } from '../github/code_intelligence' import { phabricatorCodeHost } from '../phabricator/code_intelligence' +import { findCodeViews } from './code_views' /** * Defines a type of code view a given code host can have. It tells us how to @@ -229,144 +230,11 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { codeView: HTMLElement } -/** - * Cast a Node to an HTMLElement if it has a classList. This should not be used - * if you need 100% confidence the Node is an HTMLElement. - */ -// function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { -// return !!(node as any).classList -// } - -const findCodeViews = () => (codeHosts: Observable): Observable => { - const codeViewsFromList: Observable = codeHosts.pipe( - filter(propertyIsDefined('codeViews')), - switchMap(({ codeViews }) => - of(...codeViews).pipe( - map(({ selector, ...info }) => ({ - info, - matches: document.querySelectorAll(selector), - })) - ) - ), - switchMap(({ info, matches }) => - of(...matches).pipe( - map(codeView => ({ - ...info, - codeView, - })) - ) - ) - ) - - const codeViewsFromResolver: Observable = codeHosts.pipe( - filter(propertyIsDefined('codeViewResolver')), - map(({ codeViewResolver: { selector, resolveCodeView } }) => ({ - resolveCodeView, - matches: document.querySelectorAll(selector), - })), - switchMap(({ resolveCodeView, matches }) => - of(...matches).pipe( - map(codeView => ({ - ...resolveCodeView(codeView), - codeView, - })) - ) - ) - ) - - // const codeViewsFromResolver = new Observable(observer => { - // if (!codeHost.codeViewResolver) { - // return - // } - // - // const elements = document.querySelectorAll(codeHost.codeViewResolver.selector) - // for (const elem of elements) { - // const info = codeHost.codeViewResolver.resolveCodeView(elem) - // - // observer.next({ ...info, codeView: elem }) - // } - // }) - // - // const possibleLazyLoadedCodeViews = new Subject() - // - // const mutationObserver = new MutationObserver(mutations => { - // for (const mutation of mutations) { - // console.log('mut', mutation) - // for (const node of mutation.addedNodes) { - // if (!naiveCheckIsHTMLElement(node)) { - // return - // } - // - // possibleLazyLoadedCodeViews.next(node) - // } - // } - // }) - // - // mutationObserver.observe(document.body, { - // // childList: true, - // subtree: true, - // attributes: false, - // characterData: false, - // }) - // - // const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazyLoadedCodeViews.pipe( - // filter(() => !!codeHost.codeViews), - // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), - // filter(propertyIsDefined('info')), - // map(({ codeView, info }) => ({ ...info, codeView })) - // ) - // - // const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazyLoadedCodeViews.pipe( - // filter(() => !!codeHost.codeViewResolver), - // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), - // filter(propertyIsDefined('info')), - // map(({ codeView, info }) => ({ ...info, codeView })) - // ) - // - // const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( - // switchMap( - // ({ codeView, ...rest }) => - // new Observable(observer => { - // const intersectionObserver = new IntersectionObserver( - // entries => { - // for (const entry of entries) { - // // `entry` is an `IntersectionObserverEntry`, - // // which has - // // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) - // // as a prop, but TS complains that it does not - // // exist. - // console.log('hello', entry) - // if ((entry as any).isIntersecting) { - // observer.next({ codeView, ...rest }) - // } - // } - // }, - // { - // rootMargin: '200px', - // threshold: 0, - // } - // ) - // intersectionObserver.observe(codeView) - // }) - // ) - // ) - - // return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( - // filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) - // ) - // - return merge(codeViewsFromList, codeViewsFromResolver).pipe( - filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) - ) -} - function handleCodeHost(codeHost: CodeHost): Subscription { const { hoverifier } = initCodeIntelligence(codeHost) - console.log('finding code views') - return of(codeHost) + return findCodeViews(codeHost) .pipe( - findCodeViews(), mergeMap(({ codeView, resolveFileInfo, ...rest }) => resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest }))) ) diff --git a/src/libs/code_intelligence/code_views.ts b/src/libs/code_intelligence/code_views.ts index 39e9541d..ea662681 100644 --- a/src/libs/code_intelligence/code_views.ts +++ b/src/libs/code_intelligence/code_views.ts @@ -1,13 +1,21 @@ import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' -import { Observable, of } from 'rxjs' -import { filter, map, switchMap } from 'rxjs/operators' +import { from, merge, Observable, of, Subject } from 'rxjs' +import { filter, map, mergeMap } from 'rxjs/operators' import { CodeHost, ResolvedCodeView } from './code_intelligence' -export const findCodeViews = () => (codeHosts: Observable): Observable => { - const codeViewsFromList: Observable = codeHosts.pipe( +/** + * Cast a Node to an HTMLElement if it has a classList. This should not be used + * if you need 100% confidence the Node is an HTMLElement. + */ +function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { + return !!(node as any).classList +} + +export const findCodeViews = (codeHost: CodeHost) => { + const codeViewsFromList: Observable = of(codeHost).pipe( filter(propertyIsDefined('codeViews')), - switchMap(({ codeViews }) => + mergeMap(({ codeViews }) => of(...codeViews).pipe( map(({ selector, ...info }) => ({ info, @@ -15,7 +23,7 @@ export const findCodeViews = () => (codeHosts: Observable): Observable })) ) ), - switchMap(({ info, matches }) => + mergeMap(({ info, matches }) => of(...matches).pipe( map(codeView => ({ ...info, @@ -25,13 +33,13 @@ export const findCodeViews = () => (codeHosts: Observable): Observable ) ) - const codeViewsFromResolver: Observable = codeHosts.pipe( + const codeViewsFromResolver: Observable = of(codeHost).pipe( filter(propertyIsDefined('codeViewResolver')), map(({ codeViewResolver: { selector, resolveCodeView } }) => ({ resolveCodeView, matches: document.querySelectorAll(selector), })), - switchMap(({ resolveCodeView, matches }) => + mergeMap(({ resolveCodeView, matches }) => of(...matches).pipe( map(codeView => ({ ...resolveCodeView(codeView), @@ -41,88 +49,75 @@ export const findCodeViews = () => (codeHosts: Observable): Observable ) ) - // const codeViewsFromResolver = new Observable(observer => { - // if (!codeHost.codeViewResolver) { - // return - // } - // - // const elements = document.querySelectorAll(codeHost.codeViewResolver.selector) - // for (const elem of elements) { - // const info = codeHost.codeViewResolver.resolveCodeView(elem) - // - // observer.next({ ...info, codeView: elem }) - // } - // }) - // - // const possibleLazyLoadedCodeViews = new Subject() - // - // const mutationObserver = new MutationObserver(mutations => { - // for (const mutation of mutations) { - // console.log('mut', mutation) - // for (const node of mutation.addedNodes) { - // if (!naiveCheckIsHTMLElement(node)) { - // return - // } - // - // possibleLazyLoadedCodeViews.next(node) - // } - // } - // }) - // - // mutationObserver.observe(document.body, { - // // childList: true, - // subtree: true, - // attributes: false, - // characterData: false, - // }) - // - // const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazyLoadedCodeViews.pipe( - // filter(() => !!codeHost.codeViews), - // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), - // filter(propertyIsDefined('info')), - // map(({ codeView, info }) => ({ ...info, codeView })) - // ) - // - // const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazyLoadedCodeViews.pipe( - // filter(() => !!codeHost.codeViewResolver), - // map(elem => ({ codeView: elem, info: codeHost.codeViews!.find(({ selector }) => elem.matches(selector)) })), - // filter(propertyIsDefined('info')), - // map(({ codeView, info }) => ({ ...info, codeView })) - // ) - // - // const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( - // switchMap( - // ({ codeView, ...rest }) => - // new Observable(observer => { - // const intersectionObserver = new IntersectionObserver( - // entries => { - // for (const entry of entries) { - // // `entry` is an `IntersectionObserverEntry`, - // // which has - // // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) - // // as a prop, but TS complains that it does not - // // exist. - // console.log('hello', entry) - // if ((entry as any).isIntersecting) { - // observer.next({ codeView, ...rest }) - // } - // } - // }, - // { - // rootMargin: '200px', - // threshold: 0, - // } - // ) - // intersectionObserver.observe(codeView) - // }) - // ) - // ) + const possibleLazilyLoadedContainers = new Subject() + + const mutationObserver = new MutationObserver(mutations => { + for (const mutation of mutations) { + if (mutation.type === 'childList' && naiveCheckIsHTMLElement(mutation.target)) { + const { target } = mutation + + possibleLazilyLoadedContainers.next(target) + } + } + }) + + mutationObserver.observe(document.body, { + childList: true, + subtree: true, + }) + + const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazilyLoadedContainers.pipe( + filter(() => !!codeHost.codeViews), + map(container => + codeHost.codeViews!.map(({ selector, ...info }) => ({ + info, + matches: container.querySelectorAll(selector), + })) + ), + mergeMap(codeViews => from(codeViews)), + mergeMap(({ matches, info }) => from(matches).pipe(map(codeView => ({ codeView, ...info })))) + ) + + const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazilyLoadedContainers.pipe( + filter(() => !!codeHost.codeViewResolver), + map(container => container.querySelectorAll(codeHost.codeViewResolver!.selector)), + mergeMap(matches => + of(...matches).pipe( + map(codeView => ({ codeView, info: codeHost.codeViewResolver!.resolveCodeView(codeView) })) + ) + ), + filter(propertyIsDefined('info')), + map(({ codeView, info }) => ({ ...info, codeView })) + ) + + const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( + mergeMap( + ({ codeView, ...rest }) => + new Observable(observer => { + const intersectionObserver = new IntersectionObserver( + entries => { + for (const entry of entries) { + // `entry` is an `IntersectionObserverEntry`, + // which has + // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) + // as a prop, but TS complains that it does not + // exist. + if ((entry as any).isIntersecting) { + observer.next({ codeView, ...rest }) + } + } + }, + { + rootMargin: '200px', + threshold: 0, + } + ) + intersectionObserver.observe(codeView) + }) + ) + ) - // return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( - // filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) - // ) - // - return merge(codeViewsFromList, codeViewsFromResolver).pipe( + return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) ) } From b03cfeb3c57e93be2ff138e9f4bfd68864aef214 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Mon, 17 Sep 2018 17:18:20 -0700 Subject: [PATCH 07/20] feat: only on mutation/intersection observer --- .../code_intelligence/code_intelligence.tsx | 5 +- src/libs/code_intelligence/code_views.ts | 149 +++++++++--------- 2 files changed, 79 insertions(+), 75 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 38cb41bb..38387c53 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -233,8 +233,11 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { function handleCodeHost(codeHost: CodeHost): Subscription { const { hoverifier } = initCodeIntelligence(codeHost) - return findCodeViews(codeHost) + // return findCodeViews(codeHost) + // .pipe( + return of(document.body) .pipe( + findCodeViews(codeHost), mergeMap(({ codeView, resolveFileInfo, ...rest }) => resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest }))) ) diff --git a/src/libs/code_intelligence/code_views.ts b/src/libs/code_intelligence/code_views.ts index ea662681..7d8682c4 100644 --- a/src/libs/code_intelligence/code_views.ts +++ b/src/libs/code_intelligence/code_views.ts @@ -1,4 +1,3 @@ -import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' import { from, merge, Observable, of, Subject } from 'rxjs' import { filter, map, mergeMap } from 'rxjs/operators' @@ -12,14 +11,58 @@ function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { return !!(node as any).classList } -export const findCodeViews = (codeHost: CodeHost) => { - const codeViewsFromList: Observable = of(codeHost).pipe( - filter(propertyIsDefined('codeViews')), - mergeMap(({ codeViews }) => - of(...codeViews).pipe( +/** + * Emits a ResolvedCodeView when it's DOM element is on or about to be on the page. + */ +const emitWhenIntersecting = () => { + const codeViewStash = new Map() + + const intersectingElements = new Subject() + + const intersectionObserver = new IntersectionObserver( + entries => { + for (const entry of entries) { + // `entry` is an `IntersectionObserverEntry`, + // which has + // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) + // as a prop, but TS complains that it does not + // exist. + if ((entry as any).isIntersecting) { + intersectingElements.next(entry.target as HTMLElement) + } + } + }, + { + rootMargin: '200px', + threshold: 0, + } + ) + + return (codeViews: Observable) => + new Observable(observer => { + codeViews.subscribe(({ codeView, ...rest }) => { + intersectionObserver.observe(codeView) + codeViewStash.set(codeView, { codeView, ...rest }) + }) + + intersectingElements + .pipe(map(element => codeViewStash.get(element)), filter(codeView => !!codeView)) + .subscribe(observer) + }) +} + +/** + * findCodeViews finds all the code views on a page given a CodeHost. It emits code views + * that are lazily loaded as well. + */ +export const findCodeViews = (codeHost: CodeHost, recursive = true) => (containers: Observable) => { + const codeViewsFromList: Observable = containers.pipe( + filter(() => !!codeHost.codeViews), + mergeMap(container => + from(codeHost.codeViews!).pipe( map(({ selector, ...info }) => ({ info, - matches: document.querySelectorAll(selector), + matches: container.querySelectorAll(selector), })) ) ), @@ -33,11 +76,11 @@ export const findCodeViews = (codeHost: CodeHost) => { ) ) - const codeViewsFromResolver: Observable = of(codeHost).pipe( - filter(propertyIsDefined('codeViewResolver')), - map(({ codeViewResolver: { selector, resolveCodeView } }) => ({ - resolveCodeView, - matches: document.querySelectorAll(selector), + const codeViewsFromResolver: Observable = containers.pipe( + filter(() => !!codeHost.codeViewResolver), + map(container => ({ + resolveCodeView: codeHost.codeViewResolver!.resolveCodeView, + matches: container.querySelectorAll(codeHost.codeViewResolver!.selector), })), mergeMap(({ resolveCodeView, matches }) => of(...matches).pipe( @@ -49,75 +92,33 @@ export const findCodeViews = (codeHost: CodeHost) => { ) ) - const possibleLazilyLoadedContainers = new Subject() + const obs = [codeViewsFromList, codeViewsFromResolver] - const mutationObserver = new MutationObserver(mutations => { - for (const mutation of mutations) { - if (mutation.type === 'childList' && naiveCheckIsHTMLElement(mutation.target)) { - const { target } = mutation + if (recursive) { + const possibleLazilyLoadedContainers = new Subject() - possibleLazilyLoadedContainers.next(target) - } - } - }) + const mutationObserver = new MutationObserver(mutations => { + for (const mutation of mutations) { + if (mutation.type === 'childList' && naiveCheckIsHTMLElement(mutation.target)) { + const { target } = mutation - mutationObserver.observe(document.body, { - childList: true, - subtree: true, - }) + possibleLazilyLoadedContainers.next(target) + } + } + }) - const lazilyLoadedCodeViewsFromCodeViewsList: Observable = possibleLazilyLoadedContainers.pipe( - filter(() => !!codeHost.codeViews), - map(container => - codeHost.codeViews!.map(({ selector, ...info }) => ({ - info, - matches: container.querySelectorAll(selector), - })) - ), - mergeMap(codeViews => from(codeViews)), - mergeMap(({ matches, info }) => from(matches).pipe(map(codeView => ({ codeView, ...info })))) - ) + mutationObserver.observe(document.body, { + childList: true, + subtree: true, + }) - const lazilyLoadedCodeViewsFromResolver: Observable = possibleLazilyLoadedContainers.pipe( - filter(() => !!codeHost.codeViewResolver), - map(container => container.querySelectorAll(codeHost.codeViewResolver!.selector)), - mergeMap(matches => - of(...matches).pipe( - map(codeView => ({ codeView, info: codeHost.codeViewResolver!.resolveCodeView(codeView) })) - ) - ), - filter(propertyIsDefined('info')), - map(({ codeView, info }) => ({ ...info, codeView })) - ) + const lazilyLoadedCodeViews = possibleLazilyLoadedContainers.pipe(findCodeViews(codeHost, false)) - const lazilyLoadedCodeViews = merge(lazilyLoadedCodeViewsFromCodeViewsList, lazilyLoadedCodeViewsFromResolver).pipe( - mergeMap( - ({ codeView, ...rest }) => - new Observable(observer => { - const intersectionObserver = new IntersectionObserver( - entries => { - for (const entry of entries) { - // `entry` is an `IntersectionObserverEntry`, - // which has - // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility) - // as a prop, but TS complains that it does not - // exist. - if ((entry as any).isIntersecting) { - observer.next({ codeView, ...rest }) - } - } - }, - { - rootMargin: '200px', - threshold: 0, - } - ) - intersectionObserver.observe(codeView) - }) - ) - ) + obs.push(lazilyLoadedCodeViews) + } - return merge(codeViewsFromList, codeViewsFromResolver, lazilyLoadedCodeViews).pipe( + return merge(...obs).pipe( + emitWhenIntersecting(), filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) ) } From 816b1017bb9e6e409fab136996c284d03d647bfc Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Mon, 17 Sep 2018 17:22:31 -0700 Subject: [PATCH 08/20] refactor: renames --- .../code_intelligence/code_intelligence.tsx | 2 -- src/libs/code_intelligence/code_views.ts | 22 +++++++++++-------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 38387c53..c5bbec45 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -233,8 +233,6 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { function handleCodeHost(codeHost: CodeHost): Subscription { const { hoverifier } = initCodeIntelligence(codeHost) - // return findCodeViews(codeHost) - // .pipe( return of(document.body) .pipe( findCodeViews(codeHost), diff --git a/src/libs/code_intelligence/code_views.ts b/src/libs/code_intelligence/code_views.ts index 7d8682c4..7beb3879 100644 --- a/src/libs/code_intelligence/code_views.ts +++ b/src/libs/code_intelligence/code_views.ts @@ -14,7 +14,7 @@ function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { /** * Emits a ResolvedCodeView when it's DOM element is on or about to be on the page. */ -const emitWhenIntersecting = () => { +const emitWhenIntersecting = (margin: number) => { const codeViewStash = new Map() const intersectingElements = new Subject() @@ -33,7 +33,7 @@ const emitWhenIntersecting = () => { } }, { - rootMargin: '200px', + rootMargin: `${margin}px`, threshold: 0, } ) @@ -55,7 +55,9 @@ const emitWhenIntersecting = () => { * findCodeViews finds all the code views on a page given a CodeHost. It emits code views * that are lazily loaded as well. */ -export const findCodeViews = (codeHost: CodeHost, recursive = true) => (containers: Observable) => { +export const findCodeViews = (codeHost: CodeHost, watchChildrenModifications = true) => ( + containers: Observable +) => { const codeViewsFromList: Observable = containers.pipe( filter(() => !!codeHost.codeViews), mergeMap(container => @@ -94,7 +96,7 @@ export const findCodeViews = (codeHost: CodeHost, recursive = true) => (containe const obs = [codeViewsFromList, codeViewsFromResolver] - if (recursive) { + if (watchChildrenModifications) { const possibleLazilyLoadedContainers = new Subject() const mutationObserver = new MutationObserver(mutations => { @@ -107,10 +109,12 @@ export const findCodeViews = (codeHost: CodeHost, recursive = true) => (containe } }) - mutationObserver.observe(document.body, { - childList: true, - subtree: true, - }) + containers.subscribe(container => + mutationObserver.observe(container, { + childList: true, + subtree: true, + }) + ) const lazilyLoadedCodeViews = possibleLazilyLoadedContainers.pipe(findCodeViews(codeHost, false)) @@ -118,7 +122,7 @@ export const findCodeViews = (codeHost: CodeHost, recursive = true) => (containe } return merge(...obs).pipe( - emitWhenIntersecting(), + emitWhenIntersecting(250), filter(({ codeView }) => !codeView.classList.contains('sg-mounted')) ) } From 20e22f513ae0748e9ddb748df20e8f924630e212 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Mon, 17 Sep 2018 18:57:13 -0700 Subject: [PATCH 09/20] feat: requestAnimationFrame --- .../code_intelligence/code_intelligence.tsx | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index c5bbec45..e6133840 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -138,7 +138,6 @@ export interface FileInfo { * @param codeHost */ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } { - console.log('INIT code intel') /** Emits when the go to definition button was clicked */ const goToDefinitionClicks = new Subject() const nextGoToDefinitionClick = (event: MouseEvent) => goToDefinitionClicks.next(event) @@ -279,25 +278,20 @@ function handleCodeHost(codeHost: CodeHost): Subscription { }) } -function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): void { +async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise { for (const codeHost of codeHosts) { const check = codeHost.check() const checking = check instanceof Promise ? check : Promise.resolve(check) - checking - .then(isCodeHost => { - if (isCodeHost) { - handleCodeHost(codeHost) - } - }) - .catch(err => { - /* noop */ - }) + const isCodeHost = await checking + if (isCodeHost) { + window.requestAnimationFrame(() => handleCodeHost(codeHost)) + } } } -export function injectCodeIntelligence(): void { +export async function injectCodeIntelligence(): Promise { const codeHosts: CodeHost[] = [githubCodeHost, phabricatorCodeHost] - injectCodeIntelligenceToCodeHosts(codeHosts) + await injectCodeIntelligenceToCodeHosts(codeHosts) } From 8e4b7992f22fba99e6f4389b6318d66dae49e40e Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 09:41:39 -0700 Subject: [PATCH 10/20] feat: requestAnimationFrame --- .../code_intelligence/code_intelligence.tsx | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index e6133840..412d9ee7 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -262,18 +262,20 @@ function handleCodeHost(codeHost: CodeHost): Subscription { const mount = getToolbarMount(codeView) - render( - + render( + , - mount + simpleProviderFns={lspViaAPIXlang} + />, + mount + ) ) }) } @@ -285,7 +287,7 @@ async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise const isCodeHost = await checking if (isCodeHost) { - window.requestAnimationFrame(() => handleCodeHost(codeHost)) + handleCodeHost(codeHost) } } } From 437ec6e97ec5746a1585b6a7a18e711204c1d8a1 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 11:31:20 -0700 Subject: [PATCH 11/20] feat: clean up subscriptions --- src/extension/scripts/inject.tsx | 12 +-- .../code_intelligence/code_intelligence.tsx | 98 +++++++++++-------- src/libs/github/file_info.ts | 28 +++--- src/libs/phabricator/util.tsx | 1 - 4 files changed, 73 insertions(+), 66 deletions(-) diff --git a/src/extension/scripts/inject.tsx b/src/extension/scripts/inject.tsx index cebbe11d..4075ccb5 100644 --- a/src/extension/scripts/inject.tsx +++ b/src/extension/scripts/inject.tsx @@ -35,7 +35,7 @@ function injectApplication(): void { const href = window.location.href - const handleGetStorage = (items: StorageItems) => { + const handleGetStorage = async (items: StorageItems) => { if (items.disableExtension) { return } @@ -99,11 +99,11 @@ function injectApplication(): void { } if (isGitHub || isPhabricator) { - featureFlags.isEnabled('newTooltips').then(enabled => { - if (enabled) { - injectCodeIntelligence() - } - }) + const enabled = await featureFlags.isEnabled('newTooltips') + if (enabled) { + const subscriptions = await injectCodeIntelligence() + window.addEventListener('unload', () => subscriptions.unsubscribe()) + } } setUseExtensions(items.useExtensions === undefined ? false : items.useExtensions) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 412d9ee7..d2573489 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -232,68 +232,80 @@ export interface ResolvedCodeView extends CodeViewWithOutSelector { function handleCodeHost(codeHost: CodeHost): Subscription { const { hoverifier } = initCodeIntelligence(codeHost) - return of(document.body) - .pipe( - findCodeViews(codeHost), - mergeMap(({ codeView, resolveFileInfo, ...rest }) => - resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest }))) + const subscriptions = new Subscription() + + subscriptions.add( + of(document.body) + .pipe( + findCodeViews(codeHost), + mergeMap(({ codeView, resolveFileInfo, ...rest }) => + resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest }))) + ) ) - ) - .subscribe(({ codeView, info, dom, adjustPosition, getToolbarMount, toolbarButtonProps }) => { - 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, - rev: part === 'base' ? info.baseRev || info.baseCommitID! : info.rev || info.commitID, - }) - - hoverifier.hoverify({ - dom, - positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), - resolveContext, - adjustPosition, - }) + .subscribe(({ codeView, info, dom, adjustPosition, getToolbarMount, toolbarButtonProps }) => { + 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, + rev: part === 'base' ? info.baseRev || info.baseCommitID! : info.rev || info.commitID, + }) + + subscriptions.add( + hoverifier.hoverify({ + dom, + positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)), + resolveContext, + adjustPosition, + }) + ) - codeView.classList.add('sg-mounted') + codeView.classList.add('sg-mounted') - if (!getToolbarMount) { - return - } + if (!getToolbarMount) { + return + } - const mount = getToolbarMount(codeView) + const mount = getToolbarMount(codeView) - window.requestAnimationFrame(() => - render( - + render( + , - mount + simpleProviderFns={lspViaAPIXlang} + />, + mount + ) ) - ) - }) + }) + ) + + return subscriptions } -async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise { +async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise { + const subscriptions = new Subscription() + for (const codeHost of codeHosts) { const check = codeHost.check() const checking = check instanceof Promise ? check : Promise.resolve(check) const isCodeHost = await checking if (isCodeHost) { - handleCodeHost(codeHost) + subscriptions.add(handleCodeHost(codeHost)) } } + + return subscriptions } -export async function injectCodeIntelligence(): Promise { +export async function injectCodeIntelligence(): Promise { const codeHosts: CodeHost[] = [githubCodeHost, phabricatorCodeHost] - await injectCodeIntelligenceToCodeHosts(codeHosts) + return await injectCodeIntelligenceToCodeHosts(codeHosts) } diff --git a/src/libs/github/file_info.ts b/src/libs/github/file_info.ts index ae6076fe..4c6c7e7f 100644 --- a/src/libs/github/file_info.ts +++ b/src/libs/github/file_info.ts @@ -47,22 +47,18 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable })) ) }), - map(info => { - console.log('TODO: determine if files have contents', info) + map(info => ({ + repoPath: info.repoPath, + filePath: info.headFilePath, + commitID: info.headCommitID, + rev: info.headRev, - return { - repoPath: info.repoPath, - filePath: info.headFilePath, - commitID: info.headCommitID, - rev: info.headRev, - - baseRepoPath: info.repoPath, - baseFilePath: info.baseFilePath || info.headFilePath, - baseCommitID: info.baseCommitID, - baseRev: info.baseRev, + baseRepoPath: info.repoPath, + baseFilePath: info.baseFilePath || info.headFilePath, + baseCommitID: info.baseCommitID, + baseRev: info.baseRev, - headHasFileContents: true, - baseHasFileContents: true, - } - }) + headHasFileContents: true, + baseHasFileContents: true, + })) ) diff --git a/src/libs/phabricator/util.tsx b/src/libs/phabricator/util.tsx index bcf4d542..cc30b572 100644 --- a/src/libs/phabricator/util.tsx +++ b/src/libs/phabricator/util.tsx @@ -292,7 +292,6 @@ export function getPhabricatorState( }) }) .catch(err => { - console.log('uhoh', err) reject(err) }) }) From 649244a4a70e0ad577dc56b5fd192c26b289a0d1 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 12:14:03 -0700 Subject: [PATCH 12/20] refactor: use rx animation from scheduler --- .../code_intelligence/code_intelligence.tsx | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index d2573489..3fa8d996 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -15,8 +15,8 @@ import { HoverMerged } from '@sourcegraph/codeintellify/lib/types' import { toPrettyBlobURL } from '@sourcegraph/codeintellify/lib/url' import * as React from 'react' import { render } from 'react-dom' -import { Observable, of, Subject, Subscription } from 'rxjs' -import { filter, map, mergeMap, withLatestFrom } from 'rxjs/operators' +import { animationFrameScheduler, Observable, of, Subject, Subscription } from 'rxjs' +import { filter, map, mergeMap, observeOn, withLatestFrom } from 'rxjs/operators' import { createJumpURLFetcher } from '../../shared/backend/lsp' import { lspViaAPIXlang } from '../../shared/backend/lsp' @@ -240,7 +240,8 @@ function handleCodeHost(codeHost: CodeHost): Subscription { findCodeViews(codeHost), mergeMap(({ codeView, resolveFileInfo, ...rest }) => resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest }))) - ) + ), + observeOn(animationFrameScheduler) ) .subscribe(({ codeView, info, dom, adjustPosition, getToolbarMount, toolbarButtonProps }) => { const resolveContext: ContextResolver = ({ part }) => ({ @@ -267,20 +268,18 @@ function handleCodeHost(codeHost: CodeHost): Subscription { const mount = getToolbarMount(codeView) - window.requestAnimationFrame(() => - render( - , - mount - ) + } + simpleProviderFns={lspViaAPIXlang} + />, + mount ) }) ) From ebb548a76cdad23637967dbd7193b256d737e235 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 12:50:35 -0700 Subject: [PATCH 13/20] refactor: Promise.resolve(maybePromise) --- src/libs/code_intelligence/code_intelligence.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index 3fa8d996..a897450d 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -291,10 +291,7 @@ async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise const subscriptions = new Subscription() for (const codeHost of codeHosts) { - const check = codeHost.check() - const checking = check instanceof Promise ? check : Promise.resolve(check) - - const isCodeHost = await checking + const isCodeHost = await Promise.resolve(codeHost.check()) if (isCodeHost) { subscriptions.add(handleCodeHost(codeHost)) } From e7f81bbe7b90a6248a51376246d82a35ff4a1693 Mon Sep 17 00:00:00 2001 From: Chris Wendt Date: Tue, 18 Sep 2018 13:14:17 -0700 Subject: [PATCH 14/20] refactor: inline await --- src/extension/scripts/inject.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/extension/scripts/inject.tsx b/src/extension/scripts/inject.tsx index 4075ccb5..f9432411 100644 --- a/src/extension/scripts/inject.tsx +++ b/src/extension/scripts/inject.tsx @@ -99,8 +99,7 @@ function injectApplication(): void { } if (isGitHub || isPhabricator) { - const enabled = await featureFlags.isEnabled('newTooltips') - if (enabled) { + if (await featureFlags.isEnabled('newTooltips')) { const subscriptions = await injectCodeIntelligence() window.addEventListener('unload', () => subscriptions.unsubscribe()) } From d0b3013b9c8b326d10034fe0ca8dd25183c6d656 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 14:12:23 -0700 Subject: [PATCH 15/20] refactor: address review comments --- src/libs/code_intelligence/code_intelligence.tsx | 7 +++++++ src/libs/code_intelligence/code_views.ts | 10 +--------- src/libs/github/code_intelligence.ts | 2 +- src/libs/phabricator/code_intelligence.ts | 10 ++++++---- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/libs/code_intelligence/code_intelligence.tsx b/src/libs/code_intelligence/code_intelligence.tsx index a897450d..88a0f503 100644 --- a/src/libs/code_intelligence/code_intelligence.tsx +++ b/src/libs/code_intelligence/code_intelligence.tsx @@ -300,6 +300,13 @@ async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise return subscriptions } +/** + * Injects all code hosts into the page. + * + * @returns A promise with a subscription containing all subscriptions for code + * intelligence. Unsubscribing will clean up subscriptions for hoverify and any + * incomplete setup requests. + */ export async function injectCodeIntelligence(): Promise { const codeHosts: CodeHost[] = [githubCodeHost, phabricatorCodeHost] diff --git a/src/libs/code_intelligence/code_views.ts b/src/libs/code_intelligence/code_views.ts index 7beb3879..3f5008d5 100644 --- a/src/libs/code_intelligence/code_views.ts +++ b/src/libs/code_intelligence/code_views.ts @@ -3,14 +3,6 @@ import { filter, map, mergeMap } from 'rxjs/operators' import { CodeHost, ResolvedCodeView } from './code_intelligence' -/** - * Cast a Node to an HTMLElement if it has a classList. This should not be used - * if you need 100% confidence the Node is an HTMLElement. - */ -function naiveCheckIsHTMLElement(node: Node): node is HTMLElement { - return !!(node as any).classList -} - /** * Emits a ResolvedCodeView when it's DOM element is on or about to be on the page. */ @@ -101,7 +93,7 @@ export const findCodeViews = (codeHost: CodeHost, watchChildrenModifications = t const mutationObserver = new MutationObserver(mutations => { for (const mutation of mutations) { - if (mutation.type === 'childList' && naiveCheckIsHTMLElement(mutation.target)) { + if (mutation.type === 'childList' && mutation.target instanceof HTMLElement) { const { target } = mutation possibleLazilyLoadedContainers.next(target) diff --git a/src/libs/github/code_intelligence.ts b/src/libs/github/code_intelligence.ts index 765244dc..e5eeed68 100644 --- a/src/libs/github/code_intelligence.ts +++ b/src/libs/github/code_intelligence.ts @@ -15,7 +15,7 @@ const diffCodeView: CodeViewWithOutSelector = { toolbarButtonProps, } -const resolveCodeView = (elem: HTMLElement) => { +const resolveCodeView = (elem: HTMLElement): CodeViewWithOutSelector => { const files = document.getElementsByClassName('file') const { filePath } = parseURL() const isSingleCodeFile = files.length === 1 && filePath && document.getElementsByClassName('diff-view').length === 0 diff --git a/src/libs/phabricator/code_intelligence.ts b/src/libs/phabricator/code_intelligence.ts index 996a0ddf..41fe2c15 100644 --- a/src/libs/phabricator/code_intelligence.ts +++ b/src/libs/phabricator/code_intelligence.ts @@ -30,8 +30,10 @@ function createMount( } } -// Gets the actual text content we care about and returns the number of characters we have stripped -// so that we can adjust accordingly. +/** + * Gets the actual text content we care about and returns the number of characters we have stripped + * so that we can adjust accordingly. + */ const getTextContent = (element: HTMLElement): { textContent: string; adjust: number } => { let textContent = element.textContent || '' let adjust = 0 @@ -128,9 +130,9 @@ export const phabCodeViews: CodeView[] = [ }, ] -function checkIsPhabricator(): Promise | boolean { +function checkIsPhabricator(): Promise { if (document.querySelector('.phabricator-wordmark')) { - return true + return Promise.resolve(true) } return new Promise(resolve => From 1e5d6ce2b3f581c588b31f77bef1f4f6aefe8440 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 14:55:10 -0700 Subject: [PATCH 16/20] feat: single code view --- src/libs/github/code_intelligence.ts | 13 ++++++++++--- src/libs/github/dom_functions.ts | 2 +- src/libs/github/file_info.ts | 24 ++++++++++++++++++++---- src/libs/github/inject.tsx | 10 +++++----- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/libs/github/code_intelligence.ts b/src/libs/github/code_intelligence.ts index e5eeed68..98a7ce03 100644 --- a/src/libs/github/code_intelligence.ts +++ b/src/libs/github/code_intelligence.ts @@ -1,6 +1,6 @@ import { CodeHost, CodeViewResolver, CodeViewWithOutSelector } from '../code_intelligence' -import { diffDomFunctions } from './dom_functions' -import { resolveDiffFileInfo } from './file_info' +import { diffDomFunctions, singleFileDOMFunctions } from './dom_functions' +import { resolveDiffFileInfo, resolveFileInfo } from './file_info' import { createCodeViewToolbarMount, parseURL } from './util' const toolbarButtonProps = { @@ -15,12 +15,19 @@ const diffCodeView: CodeViewWithOutSelector = { toolbarButtonProps, } +const singleFileCodeView: CodeViewWithOutSelector = { + dom: singleFileDOMFunctions, + getToolbarMount: createCodeViewToolbarMount, + resolveFileInfo, + toolbarButtonProps, +} + const resolveCodeView = (elem: HTMLElement): CodeViewWithOutSelector => { const files = document.getElementsByClassName('file') const { filePath } = parseURL() const isSingleCodeFile = files.length === 1 && filePath && document.getElementsByClassName('diff-view').length === 0 - return isSingleCodeFile ? diffCodeView : diffCodeView + return isSingleCodeFile ? singleFileCodeView : diffCodeView } const codeViewResolver: CodeViewResolver = { diff --git a/src/libs/github/dom_functions.ts b/src/libs/github/dom_functions.ts index 5bb3ed98..d0f26124 100644 --- a/src/libs/github/dom_functions.ts +++ b/src/libs/github/dom_functions.ts @@ -120,7 +120,7 @@ export const diffDomFunctions: DOMFunctions = { /** * Implementations of the DOM functions for GitHub blob code views */ -export const blobDOMFunctions: DOMFunctions = { +export const singleFileDOMFunctions: DOMFunctions = { getCodeElementFromTarget: getCodeCellFromTarget, getCodeElementFromLineNumber: (codeView, line, part) => { const lineNumberCell = codeView.querySelector(`td[data-line-number="${line}"]`) diff --git a/src/libs/github/file_info.ts b/src/libs/github/file_info.ts index 4c6c7e7f..ad26af4b 100644 --- a/src/libs/github/file_info.ts +++ b/src/libs/github/file_info.ts @@ -1,5 +1,6 @@ +import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' import { Observable, of, zip } from 'rxjs' -import { map, switchMap } from 'rxjs/operators' +import { filter, map, switchMap } from 'rxjs/operators' import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' import { FileInfo } from '../code_intelligence' import { getDeltaFileName, getDiffResolvedRev, parseURL } from './util' @@ -19,17 +20,16 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable return { ...rest, codeView, headFilePath, baseFilePath } }), - map(({ codeView, ...rest }) => { + map(data => { const diffResolvedRev = getDiffResolvedRev() if (!diffResolvedRev) { throw new Error('cannot determine delta info') } return { - codeView, headRev: diffResolvedRev.headCommitID, baseRev: diffResolvedRev.baseCommitID, - ...rest, + ...data, } }), switchMap(({ repoPath, headRev, baseRev, ...rest }) => { @@ -62,3 +62,19 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable baseHasFileContents: true, })) ) + +export const resolveFileInfo = (codeView: HTMLElement): Observable => + of(codeView).pipe( + map(codeView => { + const { repoPath, filePath, rev } = parseURL() + + return { codeView, 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/libs/github/inject.tsx b/src/libs/github/inject.tsx index b77efcdf..a94dab01 100644 --- a/src/libs/github/inject.tsx +++ b/src/libs/github/inject.tsx @@ -33,8 +33,8 @@ import { ContributableMenu } from 'sourcegraph/module/protocol' import { Disposable } from 'vscode-languageserver' import { GitHubBlobUrl } from '.' import storage from '../../browser/storage' -import { applyDecoration, createMessageTransports } from '../../shared/backend/extensions' import { createExtensionsContextController } from '../../shared/backend/extensions' +import { applyDecoration, createMessageTransports } from '../../shared/backend/extensions' import { createJumpURLFetcher, createLSPFromExtensions, @@ -64,7 +64,7 @@ import { useExtensions, } from '../../shared/util/context' import { featureFlags } from '../../shared/util/featureFlags' -import { blobDOMFunctions, diffDomFunctions, searchCodeSnippetDOMFunctions } from './dom_functions' +import { diffDomFunctions, searchCodeSnippetDOMFunctions, singleFileDOMFunctions } from './dom_functions' import { initSearch } from './search' import { createBlobAnnotatorMount, @@ -263,7 +263,7 @@ function injectCodeIntelligence(): void { injectBlobAnnotators(hoverifier, files, lspViaAPIXlang, extensionsContextController, extensionsController) - injectCodeSnippetAnnotator(hoverifier, getCodeCommentContainers(), '.border.rounded-1.my-2', blobDOMFunctions) + injectCodeSnippetAnnotator(hoverifier, getCodeCommentContainers(), '.border.rounded-1.my-2', singleFileDOMFunctions) injectCodeSnippetAnnotator( hoverifier, getRepoCodeSearchContainers(), @@ -664,8 +664,8 @@ function injectBlobAnnotators( .subscribe( commitID => { hoverifier.hoverify({ - dom: blobDOMFunctions, - positionEvents: of(file).pipe(findPositionsFromEvents(blobDOMFunctions)), + dom: singleFileDOMFunctions, + positionEvents: of(file).pipe(findPositionsFromEvents(singleFileDOMFunctions)), resolveContext: () => ({ repoPath, filePath: filePath!, From 13d0a6e799b12ca8b64d00f84a07749990e26b03 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 17:19:08 -0700 Subject: [PATCH 17/20] feat: search and comment code snippets --- src/libs/code_intelligence/code_views.ts | 5 +- src/libs/github/code_intelligence.ts | 58 ++++++++++++++++++++++-- src/libs/github/dom_functions.ts | 3 +- src/libs/github/file_info.ts | 38 ++++++++++++++-- src/libs/github/inject.tsx | 2 +- 5 files changed, 95 insertions(+), 11 deletions(-) diff --git a/src/libs/code_intelligence/code_views.ts b/src/libs/code_intelligence/code_views.ts index 3f5008d5..db510401 100644 --- a/src/libs/code_intelligence/code_views.ts +++ b/src/libs/code_intelligence/code_views.ts @@ -38,7 +38,10 @@ const emitWhenIntersecting = (margin: number) => { }) intersectingElements - .pipe(map(element => codeViewStash.get(element)), filter(codeView => !!codeView)) + .pipe( + map(element => codeViewStash.get(element)), + filter(codeView => !!codeView) + ) .subscribe(observer) }) } diff --git a/src/libs/github/code_intelligence.ts b/src/libs/github/code_intelligence.ts index 98a7ce03..59a6b1d4 100644 --- a/src/libs/github/code_intelligence.ts +++ b/src/libs/github/code_intelligence.ts @@ -1,6 +1,10 @@ -import { CodeHost, CodeViewResolver, CodeViewWithOutSelector } from '../code_intelligence' -import { diffDomFunctions, singleFileDOMFunctions } from './dom_functions' -import { resolveDiffFileInfo, resolveFileInfo } from './file_info' +import { AdjustmentDirection, PositionAdjuster } from '@sourcegraph/codeintellify' +import { trimStart } from 'lodash' +import { map } from 'rxjs/operators' +import { fetchBlobContentLines } from '../../shared/repo/backend' +import { CodeHost, CodeView, CodeViewResolver, CodeViewWithOutSelector } from '../code_intelligence' +import { diffDomFunctions, searchCodeSnippetDOMFunctions, singleFileDOMFunctions } from './dom_functions' +import { resolveDiffFileInfo, resolveFileInfo, resolveSnippetFileInfo } from './file_info' import { createCodeViewToolbarMount, parseURL } from './util' const toolbarButtonProps = { @@ -22,6 +26,53 @@ const singleFileCodeView: CodeViewWithOutSelector = { toolbarButtonProps, } +const searchResultCodeView: CodeView = { + selector: '.code-list-item', + dom: searchCodeSnippetDOMFunctions, + resolveFileInfo: resolveSnippetFileInfo, + toolbarButtonProps, +} + +/** + * Some code snippets get leading white space trimmed. This adjusts based on + * this. See an example here https://github.com/sourcegraph/browser-extensions/issues/188. + */ +const adjustPositionForSnippet: PositionAdjuster = ({ direction, codeView, position }) => + fetchBlobContentLines(position).pipe( + map(lines => { + const codeElement = singleFileDOMFunctions.getCodeElementFromLineNumber( + codeView, + position.line, + position.part + ) + if (!codeElement) { + throw new Error('(adjustPosition) could not find code element for line provided') + } + + const actualLine = lines[position.line - 1] + const documentLine = codeElement.textContent || '' + + const actualLeadingWhiteSpace = actualLine.length - trimStart(actualLine).length + const documentLeadingWhiteSpace = documentLine.length - trimStart(documentLine).length + + const modifier = direction === AdjustmentDirection.ActualToCodeView ? -1 : 1 + const delta = Math.abs(actualLeadingWhiteSpace - documentLeadingWhiteSpace) * modifier + + return { + line: position.line, + character: position.character + delta, + } + }) + ) + +const commentSnippetCodeView: CodeView = { + selector: '.js-comment-body', + dom: singleFileDOMFunctions, + resolveFileInfo: resolveSnippetFileInfo, + adjustPosition: adjustPositionForSnippet, + toolbarButtonProps, +} + const resolveCodeView = (elem: HTMLElement): CodeViewWithOutSelector => { const files = document.getElementsByClassName('file') const { filePath } = parseURL() @@ -47,6 +98,7 @@ function checkIsGithub(): boolean { export const githubCodeHost: CodeHost = { name: 'github', + codeViews: [searchResultCodeView, commentSnippetCodeView], codeViewResolver, check: checkIsGithub, } diff --git a/src/libs/github/dom_functions.ts b/src/libs/github/dom_functions.ts index d0f26124..7a0f4cfb 100644 --- a/src/libs/github/dom_functions.ts +++ b/src/libs/github/dom_functions.ts @@ -122,7 +122,7 @@ export const diffDomFunctions: DOMFunctions = { */ export const singleFileDOMFunctions: DOMFunctions = { getCodeElementFromTarget: getCodeCellFromTarget, - getCodeElementFromLineNumber: (codeView, line, part) => { + getCodeElementFromLineNumber: (codeView, line) => { const lineNumberCell = codeView.querySelector(`td[data-line-number="${line}"]`) if (!lineNumberCell) { return null @@ -154,7 +154,6 @@ export const searchCodeSnippetDOMFunctions: DOMFunctions = { const codeCell = lineNumberCell.nextElementSibling as HTMLTableCellElement // In blob views, the `` is the code element - console.log(codeCell) return codeCell }, getLineNumberFromCodeElement: (codeElement: HTMLElement): number => { diff --git a/src/libs/github/file_info.ts b/src/libs/github/file_info.ts index ad26af4b..ad83e3d2 100644 --- a/src/libs/github/file_info.ts +++ b/src/libs/github/file_info.ts @@ -1,9 +1,10 @@ -import { propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' +import { isDefined, propertyIsDefined } from '@sourcegraph/codeintellify/lib/helpers' import { Observable, of, zip } from 'rxjs' import { filter, map, switchMap } from 'rxjs/operators' +import { GitHubBlobUrl } from '.' import { resolveRev, retryWhenCloneInProgressError } from '../../shared/repo/backend' import { FileInfo } from '../code_intelligence' -import { getDeltaFileName, getDiffResolvedRev, parseURL } from './util' +import { getDeltaFileName, getDiffResolvedRev, getGitHubState, parseURL } from './util' export const resolveDiffFileInfo = (codeView: HTMLElement): Observable => of(codeView).pipe( @@ -65,12 +66,41 @@ export const resolveDiffFileInfo = (codeView: HTMLElement): Observable export const resolveFileInfo = (codeView: HTMLElement): Observable => of(codeView).pipe( - map(codeView => { + map(() => { const { repoPath, filePath, rev } = parseURL() - return { codeView, repoPath, filePath, rev } + return { repoPath, filePath, rev } + }), + filter(propertyIsDefined('filePath')), + switchMap(({ repoPath, rev, ...rest }) => + resolveRev({ repoPath, rev }).pipe( + retryWhenCloneInProgressError(), + map(commitID => ({ ...rest, repoPath, commitID, rev: rev || commitID })) + ) + ) + ) + +export const resolveSnippetFileInfo = (codeView: HTMLElement): Observable => + of(codeView).pipe( + map(codeView => { + const anchors = codeView.getElementsByTagName('a') + let githubState: GitHubBlobUrl | undefined + for (const anchor of anchors) { + const anchorState = getGitHubState(anchor.href) as GitHubBlobUrl + if (anchorState) { + githubState = anchorState + break + } + } + + return githubState }), + filter(isDefined), + filter(propertyIsDefined('owner')), + filter(propertyIsDefined('repoName')), + filter(propertyIsDefined('rev')), filter(propertyIsDefined('filePath')), + map(({ owner, repoName, ...rest }) => ({ repoPath: `${window.location.host}/${owner}/${repoName}`, ...rest })), switchMap(({ repoPath, rev, ...rest }) => resolveRev({ repoPath, rev }).pipe( retryWhenCloneInProgressError(), diff --git a/src/libs/github/inject.tsx b/src/libs/github/inject.tsx index a94dab01..67fa47b1 100644 --- a/src/libs/github/inject.tsx +++ b/src/libs/github/inject.tsx @@ -422,7 +422,7 @@ function injectCodeSnippetAnnotator( const repoPath = `${window.location.host}/${owner}/${repoName}` const mount = document.createElement('div') - mount.style.display = 'none' + // mount.style.display = 'none' mount.className = 'sourcegraph-app-annotator' filePathContainer.appendChild(mount) From 371e09aa00fd8140c080473d2d29f5ba2b1b71a4 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 17:21:01 -0700 Subject: [PATCH 18/20] feat: search and comment code snippets --- src/libs/github/code_intelligence.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libs/github/code_intelligence.ts b/src/libs/github/code_intelligence.ts index 59a6b1d4..23e05a99 100644 --- a/src/libs/github/code_intelligence.ts +++ b/src/libs/github/code_intelligence.ts @@ -26,13 +26,6 @@ const singleFileCodeView: CodeViewWithOutSelector = { toolbarButtonProps, } -const searchResultCodeView: CodeView = { - selector: '.code-list-item', - dom: searchCodeSnippetDOMFunctions, - resolveFileInfo: resolveSnippetFileInfo, - toolbarButtonProps, -} - /** * Some code snippets get leading white space trimmed. This adjusts based on * this. See an example here https://github.com/sourcegraph/browser-extensions/issues/188. @@ -65,6 +58,14 @@ const adjustPositionForSnippet: PositionAdjuster = ({ direction, codeView, posit }) ) +const searchResultCodeView: CodeView = { + selector: '.code-list-item', + dom: searchCodeSnippetDOMFunctions, + adjustPosition: adjustPositionForSnippet, + resolveFileInfo: resolveSnippetFileInfo, + toolbarButtonProps, +} + const commentSnippetCodeView: CodeView = { selector: '.js-comment-body', dom: singleFileDOMFunctions, From 087ed54b3ee0a8cd555fba0d8efb7f20e6f6e83a Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Tue, 18 Sep 2018 17:34:12 -0700 Subject: [PATCH 19/20] feat: put behind feature flag --- src/browser/types.ts | 2 ++ src/extension/scripts/inject.tsx | 2 +- src/libs/github/inject.tsx | 2 +- src/libs/phabricator/app.tsx | 2 +- src/libs/phabricator/extension.tsx | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/browser/types.ts b/src/browser/types.ts index e1b4f09d..e7c0c083 100644 --- a/src/browser/types.ts +++ b/src/browser/types.ts @@ -12,10 +12,12 @@ export interface PhabricatorMapping { */ export interface FeatureFlags { newTooltips: boolean + newInject: boolean } export const featureFlagDefaults: FeatureFlags = { newTooltips: true, + newInject: false, } // TODO(chris) Switch to Partial to eliminate bugs caused by diff --git a/src/extension/scripts/inject.tsx b/src/extension/scripts/inject.tsx index f9432411..157c4bd4 100644 --- a/src/extension/scripts/inject.tsx +++ b/src/extension/scripts/inject.tsx @@ -99,7 +99,7 @@ function injectApplication(): void { } if (isGitHub || isPhabricator) { - if (await featureFlags.isEnabled('newTooltips')) { + if (await featureFlags.isEnabled('newInject')) { const subscriptions = await injectCodeIntelligence() window.addEventListener('unload', () => subscriptions.unsubscribe()) } diff --git a/src/libs/github/inject.tsx b/src/libs/github/inject.tsx index 67fa47b1..f4ae59f4 100644 --- a/src/libs/github/inject.tsx +++ b/src/libs/github/inject.tsx @@ -274,7 +274,7 @@ function injectCodeIntelligence(): void { function inject(): void { featureFlags - .isEnabled('newTooltips') + .isEnabled('newInject') .then(isEnabled => { if (!isEnabled) { injectCodeIntelligence() diff --git a/src/libs/phabricator/app.tsx b/src/libs/phabricator/app.tsx index f2d37b3c..55e8bc6a 100644 --- a/src/libs/phabricator/app.tsx +++ b/src/libs/phabricator/app.tsx @@ -22,7 +22,7 @@ export function injectPhabricatorApplication(): void { function injectModules(): void { featureFlags - .isEnabled('newTooltips') + .isEnabled('newInject') .then(enabled => { if (enabled) { return diff --git a/src/libs/phabricator/extension.tsx b/src/libs/phabricator/extension.tsx index 3c16e32d..5cd89bc3 100644 --- a/src/libs/phabricator/extension.tsx +++ b/src/libs/phabricator/extension.tsx @@ -15,7 +15,7 @@ function injectModules(): void { document.body.appendChild(extensionMarker) featureFlags - .isEnabled('newTooltips') + .isEnabled('newInject') .then(enabled => { if (enabled) { injectCodeIntelligence() From a4a2e3030ba7c12a2c98e0b509d2a6ccb2556944 Mon Sep 17 00:00:00 2001 From: Isaac Snow Date: Wed, 19 Sep 2018 09:33:43 -0700 Subject: [PATCH 20/20] chore: tslint --- src/libs/phabricator/extension.tsx | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/libs/phabricator/extension.tsx b/src/libs/phabricator/extension.tsx index 5cd89bc3..c4871040 100644 --- a/src/libs/phabricator/extension.tsx +++ b/src/libs/phabricator/extension.tsx @@ -8,23 +8,17 @@ import { injectPhabricatorBlobAnnotators } from './inject_old' import { expanderListen, metaClickOverride, setupPageLoadListener } from './util' // NOTE: injectModules is idempotent, so safe to call multiple times on the same page. -function injectModules(): void { +async function injectModules(): Promise { const extensionMarker = document.createElement('div') extensionMarker.id = 'sourcegraph-app-background' extensionMarker.style.display = 'none' document.body.appendChild(extensionMarker) - featureFlags - .isEnabled('newInject') - .then(enabled => { - if (enabled) { - injectCodeIntelligence() - return - } - - injectPhabricatorBlobAnnotators().catch(e => console.error(e)) - }) - .catch(err => console.error('could not get feature flag', err)) + if (await featureFlags.isEnabled('newInject')) { + await injectCodeIntelligence() + } else { + await injectPhabricatorBlobAnnotators() + } } export function init(): void { @@ -37,7 +31,7 @@ export function init(): void { // passed the bundle url. Legacy Phabricator extensions inject CSS via the loader.js script // so we do not need to do this here. if (!window.SOURCEGRAPH_BUNDLE_URL && !window.localStorage.getItem('SOURCEGRAPH_BUNDLE_URL')) { - injectModules() + injectModules().catch(err => console.error('Unable to inject modules', err)) metaClickOverride() expanderListen() return @@ -57,7 +51,7 @@ export function init(): void { setSourcegraphUrl(sourcegraphUrl) expanderListen() metaClickOverride() - injectModules() + injectModules().catch(err => console.error('Unable to inject modules', err)) }) .catch(e => { console.error(e)