From 36790f976f0cee52bb4d3ecb4ab3eb08c83a1691 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Mon, 28 Sep 2020 21:03:38 +0200 Subject: [PATCH] Refactor user feedbacks (#777) * Extract the task handler from the model Refactor user logger * Avoid focus capture by suspend modal * Propagate error not related to authentication --- package.json | 3 + src/commandsAndMenu.tsx | 225 +++++-- src/components/Alert.tsx | 34 +- src/components/BranchMenu.tsx | 172 ++--- src/components/Feedback.tsx | 132 ++++ src/components/GitPanel.tsx | 202 ++---- src/components/HistorySideBar.tsx | 6 - src/components/NewBranchDialog.tsx | 166 +---- src/components/PastCommitNode.tsx | 6 - src/components/ResetRevertDialog.tsx | 253 ++------ src/components/SinglePastCommitInfo.tsx | 26 +- src/components/SuspendModal.tsx | 7 +- src/components/Toolbar.tsx | 179 ++---- src/index.ts | 2 +- src/logger.ts | 40 ++ src/model.ts | 589 ++++++++---------- src/taskhandler.ts | 111 ++++ src/tokens.ts | 27 +- src/widgets/GitCloneForm.ts | 36 ++ src/widgets/GitWidget.tsx | 42 +- src/widgets/StatusWidget.ts | 8 +- src/widgets/gitClone.tsx | 136 +--- src/widgets/gitPushPull.ts | 119 ---- tests/test-components/BranchMenu.spec.tsx | 35 +- tests/test-components/GitPanel.spec.tsx | 2 + tests/test-components/HistorySideBar.spec.tsx | 3 +- tests/test-components/PastCommitNode.spec.tsx | 3 +- tests/test-components/Toolbar.spec.tsx | 37 +- 28 files changed, 1117 insertions(+), 1484 deletions(-) create mode 100644 src/components/Feedback.tsx create mode 100644 src/logger.ts create mode 100644 src/taskhandler.ts create mode 100644 src/widgets/GitCloneForm.ts delete mode 100644 src/widgets/gitPushPull.ts diff --git a/package.json b/package.json index 657ae4bda..ac1da94ee 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,10 @@ "@jupyterlab/terminal": "^2.0.0", "@jupyterlab/ui-components": "^2.0.0", "@lumino/collections": "^1.2.3", + "@lumino/commands": "^1.11.0", + "@lumino/coreutils": "^1.5.0", "@lumino/polling": "^1.0.4", + "@lumino/signaling": "^1.4.0", "@lumino/widgets": "^1.11.1", "@material-ui/core": "^4.8.2", "@material-ui/icons": "^4.5.1", diff --git a/src/commandsAndMenu.tsx b/src/commandsAndMenu.tsx index b8d506d6b..2e3479643 100644 --- a/src/commandsAndMenu.tsx +++ b/src/commandsAndMenu.tsx @@ -21,12 +21,13 @@ import { RenderMimeProvider } from './components/diff/Diff'; import { getRefValue, IDiffContext } from './components/diff/model'; +import { AUTH_ERROR_MESSAGES } from './git'; +import { logger } from './logger'; import { GitExtension } from './model'; import { diffIcon } from './style/icons'; -import { Git } from './tokens'; +import { Git, Level } from './tokens'; import { GitCredentialsForm } from './widgets/CredentialsBox'; -import { doGitClone } from './widgets/gitClone'; -import { GitPullPushDialog, Operation } from './widgets/gitPushPull'; +import { GitCloneForm } from './widgets/GitCloneForm'; const RESOURCES = [ { @@ -39,6 +40,26 @@ const RESOURCES = [ } ]; +interface IGitCloneArgs { + /** + * Path in which to clone the Git repository + */ + path: string; + /** + * Git repository url + */ + url: string; +} + +/** + * Git operations requiring authentication + */ +enum Operation { + Clone = 'Clone', + Pull = 'Pull', + Push = 'Push' +} + /** * The command IDs used by the git plugin. */ @@ -133,8 +154,28 @@ export function addCommands( }); if (result.button.accept) { - await model.init(currentPath); - model.pathRepository = currentPath; + logger.log({ + message: 'Initializing...', + level: Level.RUNNING + }); + try { + await model.init(currentPath); + model.pathRepository = currentPath; + logger.log({ + message: 'Git repository initialized.', + level: Level.SUCCESS + }); + } catch (error) { + console.error( + 'Encountered an error when initializing the repository. Error: ', + error + ); + logger.log({ + message: 'Failed to initialize the Git repository', + level: Level.ERROR, + error + }); + } } }, isEnabled: () => model.pathRepository === null @@ -208,8 +249,41 @@ export function addCommands( caption: 'Clone a repository from a URL', isEnabled: () => model.pathRepository === null, execute: async () => { - await doGitClone(model, fileBrowser.model.path); - fileBrowser.model.refresh(); + const result = await showDialog({ + title: 'Clone a repo', + body: new GitCloneForm(), + focusNodeSelector: 'input', + buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'CLONE' })] + }); + + if (result.button.accept && result.value) { + logger.log({ + level: Level.RUNNING, + message: 'Cloning...' + }); + try { + await Private.showGitOperationDialog( + model, + Operation.Clone, + { path: fileBrowser.model.path, url: result.value } + ); + logger.log({ + message: 'Successfully cloned', + level: Level.SUCCESS + }); + await fileBrowser.model.refresh(); + } catch (error) { + console.error( + 'Encountered an error when cloning the repository. Error: ', + error + ); + logger.log({ + message: 'Failed to clone', + level: Level.ERROR, + error + }); + } + } } }); @@ -229,13 +303,27 @@ export function addCommands( caption: 'Push code to remote repository', isEnabled: () => model.pathRepository !== null, execute: async () => { - await Private.showGitOperationDialog(model, Operation.Push).catch( - reason => { - console.error( - `Encountered an error when pushing changes. Error: ${reason}` - ); - } - ); + logger.log({ + level: Level.RUNNING, + message: 'Pushing...' + }); + try { + await Private.showGitOperationDialog(model, Operation.Push); + logger.log({ + message: 'Successfully pushed', + level: Level.SUCCESS + }); + } catch (error) { + console.error( + 'Encountered an error when pushing changes. Error: ', + error + ); + logger.log({ + message: 'Failed to push', + level: Level.ERROR, + error + }); + } } }); @@ -245,13 +333,27 @@ export function addCommands( caption: 'Pull latest code from remote repository', isEnabled: () => model.pathRepository !== null, execute: async () => { - await Private.showGitOperationDialog(model, Operation.Pull).catch( - reason => { - console.error( - `Encountered an error when pulling changes. Error: ${reason}` - ); - } - ); + logger.log({ + level: Level.RUNNING, + message: 'Pulling...' + }); + try { + await Private.showGitOperationDialog(model, Operation.Pull); + logger.log({ + message: 'Successfully pulled', + level: Level.SUCCESS + }); + } catch (error) { + console.error( + 'Encountered an error when pulling changes. Error: ', + error + ); + logger.log({ + message: 'Failed to pull', + level: Level.ERROR, + error + }); + } } }); @@ -515,44 +617,69 @@ export function createGitMenu(commands: CommandRegistry): Menu { /* eslint-disable no-inner-declarations */ namespace Private { /** - * Displays an error dialog when a Git operation fails. + * Handle Git operation that may require authentication. * * @private * @param model - Git extension model * @param operation - Git operation name + * @param args - Git operation arguments + * @param authentication - Git authentication information + * @param retry - Is this operation retried? * @returns Promise for displaying a dialog */ - export async function showGitOperationDialog( + export async function showGitOperationDialog( model: GitExtension, - operation: Operation + operation: Operation, + args?: T, + authentication?: Git.IAuth, + retry = false ): Promise { - const title = `Git ${operation}`; - let result = await showDialog({ - title: title, - body: new GitPullPushDialog(model, operation), - buttons: [Dialog.okButton({ label: 'DISMISS' })] - }); - let retry = false; - while (!result.button.accept) { - const credentials = await showDialog({ - title: 'Git credentials required', - body: new GitCredentialsForm( - 'Enter credentials for remote repository', - retry ? 'Incorrect username or password.' : '' - ), - buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'OK' })] - }); - - if (!credentials.button.accept) { - break; + try { + // the Git action + switch (operation) { + case Operation.Clone: + // eslint-disable-next-line no-case-declarations + const { path, url } = (args as any) as IGitCloneArgs; + await model.clone(path, url, authentication); + break; + case Operation.Pull: + await model.pull(authentication); + break; + case Operation.Push: + await model.push(authentication); + break; + default: + return; } + } catch (error) { + if ( + AUTH_ERROR_MESSAGES.some( + errorMessage => error.message.indexOf(errorMessage) > -1 + ) + ) { + // If the error is an authentication error, ask the user credentials + const credentials = await showDialog({ + title: 'Git credentials required', + body: new GitCredentialsForm( + 'Enter credentials for remote repository', + retry ? 'Incorrect username or password.' : '' + ) + }); - result = await showDialog({ - title: title, - body: new GitPullPushDialog(model, operation, credentials.value), - buttons: [Dialog.okButton({ label: 'DISMISS' })] - }); - retry = true; + if (credentials.button.accept) { + // Retry the operation if the user provides its credentials + return await showGitOperationDialog( + model, + operation, + args, + credentials.value, + true + ); + } + } + // Throw the error if it cannot be handled or + // if the user did not accept to provide its credentials + throw error; } } } diff --git a/src/components/Alert.tsx b/src/components/Alert.tsx index b41a07e1d..4e47b2256 100644 --- a/src/components/Alert.tsx +++ b/src/components/Alert.tsx @@ -1,9 +1,10 @@ -import * as React from 'react'; +import { showErrorMessage } from '@jupyterlab/apputils'; +import { Button } from '@material-ui/core'; import Portal from '@material-ui/core/Portal'; -import Snackbar from '@material-ui/core/Snackbar'; import Slide from '@material-ui/core/Slide'; -import { default as MuiAlert } from '@material-ui/lab/Alert'; -import { Severity } from '../tokens'; +import Snackbar from '@material-ui/core/Snackbar'; +import { Color, default as MuiAlert } from '@material-ui/lab/Alert'; +import * as React from 'react'; /** * Returns a React component for "sliding-in" an alert. @@ -30,10 +31,15 @@ export interface IAlertProps { */ message: string; + /** + * Error object + */ + error?: Error; + /** * Alert severity. */ - severity?: Severity; + severity?: Color; /** * Alert duration (in milliseconds). @@ -91,7 +97,23 @@ export class Alert extends React.Component { onClick={this._onClick} onClose={this._onClose} > - + { + showErrorMessage('Error', this.props.error); + }} + > + SHOW + + ) + } + variant="filled" + severity={severity} + > {this.props.message || '(missing message)'} diff --git a/src/components/BranchMenu.tsx b/src/components/BranchMenu.tsx index 6cf0ca79d..550a49379 100644 --- a/src/components/BranchMenu.tsx +++ b/src/components/BranchMenu.tsx @@ -5,6 +5,7 @@ import ClearIcon from '@material-ui/icons/Clear'; import * as React from 'react'; import { FixedSizeList, ListChildComponentProps } from 'react-window'; import { classes } from 'typestyle'; +import { Logger } from '../logger'; import { activeListItemClass, filterClass, @@ -16,11 +17,8 @@ import { newBranchButtonClass, wrapperClass } from '../style/BranchMenu'; -import { Git, IGitExtension, ILogMessage } from '../tokens'; -import { sleep } from '../utils'; -import { Alert } from './Alert'; +import { Git, IGitExtension, Level } from '../tokens'; import { NewBranchDialog } from './NewBranchDialog'; -import { SuspendModal } from './SuspendModal'; const CHANGES_ERR_MSG = 'The current branch contains files with uncommitted changes. Please commit or discard these changes before switching to or creating another branch.'; @@ -32,10 +30,16 @@ const MAX_HEIGHT = 400; // Maximal HTML element height for the branches list * Callback invoked upon encountering an error when switching branches. * * @private - * @param err - error + * @param error - error + * @param logger - the logger */ -function onBranchError(err: any): void { - if (err.message.includes('following files would be overwritten')) { +function onBranchError(error: any, logger: Logger): void { + if (error.message.includes('following files would be overwritten')) { + // Empty log message to hide the executing alert + logger.log({ + message: '', + level: Level.INFO + }); showDialog({ title: 'Unable to switch branch', body: ( @@ -45,7 +49,7 @@ function onBranchError(err: any): void { switching:

- {err.message + {error.message .split('\n') .slice(1, -3) .map(renderFileName)} @@ -59,7 +63,11 @@ function onBranchError(err: any): void { buttons: [Dialog.okButton({ label: 'Dismiss' })] }); } else { - showErrorMessage('Error switching branch', err.message); + logger.log({ + level: Level.ERROR, + message: 'Failed to switch branch.', + error + }); } } @@ -79,19 +87,19 @@ function renderFileName(filename: string): React.ReactElement { */ export interface IBranchMenuProps { /** - * Git extension data model. + * Boolean indicating whether branching is disabled. */ - model: IGitExtension; + branching: boolean; /** - * Boolean indicating whether branching is disabled. + * Extension logger */ - branching: boolean; + logger: Logger; /** - * Boolean indicating whether to enable UI suspension. + * Git extension data model. */ - suspend: boolean; + model: IGitExtension; } /** @@ -99,39 +107,24 @@ export interface IBranchMenuProps { */ export interface IBranchMenuState { /** - * Menu filter. + * Current branch name. */ - filter: string; + current: string; /** * Boolean indicating whether to show a dialog to create a new branch. */ branchDialog: boolean; - /** - * Current branch name. - */ - current: string; - /** * Current list of branches. */ branches: Git.IBranch[]; /** - * Boolean indicating whether UI interaction should be suspended (e.g., due to pending command). - */ - suspend: boolean; - - /** - * Boolean indicating whether to show an alert. - */ - alert: boolean; - - /** - * Log message. + * Menu filter. */ - log: ILogMessage; + filter: string; } /** @@ -156,13 +149,7 @@ export class BranchMenu extends React.Component< filter: '', branchDialog: false, current: repo ? this.props.model.currentBranch.name : '', - branches: repo ? this.props.model.branches : [], - suspend: false, - alert: false, - log: { - severity: 'info', - message: '' - } + branches: repo ? this.props.model.branches : [] }; } @@ -191,7 +178,6 @@ export class BranchMenu extends React.Component< {this._renderFilter()} {this._renderBranchList()} {this._renderNewBranchDialog()} - {this._renderFeedback()} ); } @@ -304,36 +290,14 @@ export class BranchMenu extends React.Component< private _renderNewBranchDialog(): React.ReactElement { return ( ); } - /** - * Renders a component to provide UI feedback. - * - * @returns React element - */ - private _renderFeedback(): React.ReactElement { - return ( - - - - - ); - } - /** * Adds model listeners. */ @@ -364,31 +328,6 @@ export class BranchMenu extends React.Component< }); } - /** - * Sets the suspension state. - * - * @param bool - boolean indicating whether to suspend UI interaction - */ - private _suspend(bool: boolean): void { - if (this.props.suspend) { - this.setState({ - suspend: bool - }); - } - } - - /** - * Sets the current component log message. - * - * @param msg - log message - */ - private _log(msg: ILogMessage): void { - this.setState({ - alert: true, - log: msg - }); - } - /** * Callback invoked upon a change to the menu filter. * @@ -459,60 +398,21 @@ export class BranchMenu extends React.Component< branchname: branch }; - self._log({ - severity: 'info', + self.props.logger.log({ + level: Level.RUNNING, message: 'Switching branch...' }); - self._suspend(true); - let result: Array; try { - result = await Promise.all([ - sleep(1000), - self.props.model.checkout(opts) - ]); + await self.props.model.checkout(opts); } catch (err) { - self._suspend(false); - self._log({ - severity: 'error', - message: 'Failed to switch branch.' - }); - return onBranchError(err); + return onBranchError(err, self.props.logger); } - self._suspend(false); - const res = result[1] as Git.ICheckoutResult; - if (res.code !== 0) { - self._log({ - severity: 'error', - message: 'Failed to switch branch.' - }); - showErrorMessage('Error switching branch', res.message); - return; - } - self._log({ - severity: 'success', + + self.props.logger.log({ + level: Level.SUCCESS, message: 'Switched branch.' }); } } - - /** - * Callback invoked upon clicking on the feedback modal. - * - * @param event - event object - */ - private _onFeedbackModalClick = (): void => { - this._suspend(false); - }; - - /** - * Callback invoked upon closing a feedback alert. - * - * @param event - event object - */ - private _onFeedbackAlertClose = (): void => { - this.setState({ - alert: false - }); - }; } diff --git a/src/components/Feedback.tsx b/src/components/Feedback.tsx new file mode 100644 index 000000000..24c76203c --- /dev/null +++ b/src/components/Feedback.tsx @@ -0,0 +1,132 @@ +import { ISettingRegistry } from '@jupyterlab/settingregistry'; +import { Color } from '@material-ui/lab/Alert'; +import * as React from 'react'; +import { ILogMessage, Level } from '../tokens'; +import { Alert } from './Alert'; +import { SuspendModal } from './SuspendModal'; + +const LEVEL_TO_SEVERITY: Map = new Map([ + [Level.ERROR, 'error'], + [Level.WARNING, 'warning'], + [Level.SUCCESS, 'success'], + [Level.INFO, 'info'], + [Level.RUNNING, 'info'] +]); + +const VISUAL_DELAY = 1000; // in ms + +export interface IFeedbackProps { + /** + * Alert + */ + log: ILogMessage; + + /** + * Extension settings + */ + settings: ISettingRegistry.ISettings; +} + +export interface IFeedbackState { + /** + * Overlay visibility + */ + blockUI: boolean; + + /** + * Log message stack + */ + logStack: ILogMessage[]; + + /** + * Last time the feedback message was changed + */ + lastUpdate: number; + + /** + * Alert visibility + */ + showAlert: boolean; +} + +/** + * Component to handle logger user feedback + */ +export class Feedback extends React.Component { + constructor(props: IFeedbackProps) { + super(props); + + this.state = { + blockUI: false, + lastUpdate: Date.now() - VISUAL_DELAY, + logStack: [], + showAlert: false + }; + } + + static getDerivedStateFromProps( + props: IFeedbackProps, + state: IFeedbackState + ): IFeedbackState { + const latestLog = state.logStack[state.logStack.length - 1]; + const now = Date.now(); + if (props.log !== latestLog) { + if (now - state.lastUpdate > VISUAL_DELAY) { + state.logStack.shift(); + } + if (latestLog && props.log.level > latestLog.level) { + // Higher level takes over + state.logStack.splice(0, 1, props.log); + state.lastUpdate = now; + } else { + state.logStack.push(props.log); + } + state.blockUI = props.settings.composite[ + 'blockWhileCommandExecutes' + ] as boolean; + state.showAlert = true; + } + return state; + } + + render() { + if (this.state.logStack.length > 1) { + setTimeout(() => { + if (this.state.logStack.length > 1) { + this.setState({ + blockUI: this.props.settings.composite[ + 'blockWhileCommandExecutes' + ] as boolean, + logStack: this.state.logStack.slice(1), + lastUpdate: Date.now(), + showAlert: true + }); + } + }, VISUAL_DELAY); + } + + const log = this.state.logStack[0]; + + return ( + + { + this.setState({ blockUI: false }); + }} + /> + this.setState({ showAlert: false })} + /> + + ); + } +} diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index b8026c339..e7fff87c0 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -1,4 +1,4 @@ -import { showDialog, showErrorMessage } from '@jupyterlab/apputils'; +import { showDialog } from '@jupyterlab/apputils'; import { PathExt } from '@jupyterlab/coreutils'; import { FileBrowserModel } from '@jupyterlab/filebrowser'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; @@ -8,6 +8,7 @@ import Tab from '@material-ui/core/Tab'; import Tabs from '@material-ui/core/Tabs'; import * as React from 'react'; import { CommandIDs } from '../commandsAndMenu'; +import { Logger } from '../logger'; import { GitExtension } from '../model'; import { panelWrapperClass, @@ -18,14 +19,11 @@ import { tabsClass, warningTextClass } from '../style/GitPanel'; -import { Git, ILogMessage } from '../tokens'; -import { sleep } from '../utils'; +import { Git, ILogMessage, Level } from '../tokens'; import { GitAuthorForm } from '../widgets/AuthorBox'; -import { Alert } from './Alert'; import { CommitBox } from './CommitBox'; import { FileList } from './FileList'; import { HistorySideBar } from './HistorySideBar'; -import { SuspendModal } from './SuspendModal'; import { Toolbar } from './Toolbar'; /** @@ -33,24 +31,29 @@ import { Toolbar } from './Toolbar'; */ export interface IGitPanelProps { /** - * Git extension data model. + * Jupyter App commands registry */ - model: GitExtension; + commands: CommandRegistry; /** - * Jupyter App commands registry + * File browser model. */ - commands: CommandRegistry; + filebrowser: FileBrowserModel; /** - * Git extension settings. + * Extension logger */ - settings: ISettingRegistry.ISettings; + logger: Logger; /** - * File browser model. + * Git extension data model. */ - filebrowser: FileBrowserModel; + model: GitExtension; + + /** + * Git extension settings. + */ + settings: ISettingRegistry.ISettings; } /** @@ -86,21 +89,6 @@ export interface IGitPanelState { * Panel tab identifier. */ tab: number; - - /** - * Boolean indicating whether UI interaction should be suspended (e.g., due to pending command). - */ - suspend: boolean; - - /** - * Boolean indicating whether to show an alert. - */ - alert: boolean; - - /** - * Log message. - */ - log: ILogMessage; } /** @@ -115,19 +103,14 @@ export class GitPanel extends React.Component { */ constructor(props: IGitPanelProps) { super(props); + this.state = { branches: [], currentBranch: '', files: [], inGitRepository: false, pastCommits: [], - tab: 0, - suspend: false, - alert: false, - log: { - severity: 'info', - message: '' - } + tab: 0 }; } @@ -207,17 +190,14 @@ export class GitPanel extends React.Component { * @returns a promise which commits the files */ commitMarkedFiles = async (message: string): Promise => { - this._suspend(true); - - this._log({ - severity: 'info', + this.props.logger.log({ + level: Level.RUNNING, message: 'Staging files...' }); await this.props.model.reset(); await this.props.model.add(...this._markedFiles.map(file => file.to)); await this.commitStagedFiles(message); - this._suspend(false); }; /** @@ -227,50 +207,38 @@ export class GitPanel extends React.Component { * @returns a promise which commits the files */ commitStagedFiles = async (message: string): Promise => { - let res: boolean; if (!message) { return; } + + const errorLog: ILogMessage = { + level: Level.ERROR, + message: 'Failed to commit changes.' + }; + try { - res = await this._hasIdentity(this.props.model.pathRepository); - } catch (err) { - this._log({ - severity: 'error', - message: 'Failed to commit changes.' - }); - console.error(err); - showErrorMessage('Fail to commit', err); - return; - } - if (!res) { - this._log({ - severity: 'error', - message: 'Failed to commit changes.' + const res = await this._hasIdentity(this.props.model.pathRepository); + + if (!res) { + this.props.logger.log(errorLog); + return; + } + + this.props.logger.log({ + level: Level.RUNNING, + message: 'Committing changes...' }); - return; - } - this._log({ - severity: 'info', - message: 'Committing changes...' - }); - this._suspend(true); - try { - await Promise.all([sleep(1000), this.props.model.commit(message)]); - } catch (err) { - this._suspend(false); - this._log({ - severity: 'error', - message: 'Failed to commit changes.' + + await this.props.model.commit(message); + + this.props.logger.log({ + level: Level.SUCCESS, + message: 'Committed changes.' }); - console.error(err); - showErrorMessage('Fail to commit', err); - return; + } catch (error) { + console.error(error); + this.props.logger.log({ ...errorLog, error }); } - this._suspend(false); - this._log({ - severity: 'success', - message: 'Committed changes.' - }); }; /** @@ -285,7 +253,6 @@ export class GitPanel extends React.Component { {this._renderToolbar()} {this._renderMain()} - {this._renderFeedback()} ) : ( this._renderWarning() @@ -306,13 +273,11 @@ export class GitPanel extends React.Component { ); return ( ); } @@ -411,38 +376,10 @@ export class GitPanel extends React.Component { commits={this.state.pastCommits} model={this.props.model} commands={this.props.commands} - suspend={ - this.props.settings.composite['blockWhileCommandExecutes'] as boolean - } /> ); } - /** - * Renders a component to provide UI feedback. - * - * @returns React element - */ - private _renderFeedback(): React.ReactElement { - return ( - - - - - ); - } - /** * Renders a panel for prompting a user to find a Git repository. * @@ -491,31 +428,6 @@ export class GitPanel extends React.Component { ); } - /** - * Sets the suspension state. - * - * @param bool - boolean indicating whether to suspend UI interaction - */ - private _suspend(bool: boolean): void { - if (this.props.settings.composite['blockWhileCommandExecutes']) { - this.setState({ - suspend: bool - }); - } - } - - /** - * Sets the current component log message. - * - * @param msg - log message - */ - private _log(msg: ILogMessage): void { - this.setState({ - alert: true, - log: msg - }); - } - /** * Callback invoked upon changing the active panel tab. * @@ -545,26 +457,6 @@ export class GitPanel extends React.Component { } }; - /** - * Callback invoked upon clicking on the feedback modal. - * - * @param event - event object - */ - private _onFeedbackModalClick = (): void => { - this._suspend(false); - }; - - /** - * Callback invoked upon closing a feedback alert. - * - * @param event - event object - */ - private _onFeedbackAlertClose = (): void => { - this.setState({ - alert: false - }); - }; - /** * Determines whether a user has a known Git identity. * diff --git a/src/components/HistorySideBar.tsx b/src/components/HistorySideBar.tsx index dea917116..ce6a98219 100644 --- a/src/components/HistorySideBar.tsx +++ b/src/components/HistorySideBar.tsx @@ -28,11 +28,6 @@ export interface IHistorySideBarProps { * Jupyter App commands registry */ commands: CommandRegistry; - - /** - * Boolean indicating whether to enable UI suspension. - */ - suspend: boolean; } /** @@ -52,7 +47,6 @@ export const HistorySideBar: React.FunctionComponent = ( branches={props.branches} model={props.model} commands={props.commands} - suspend={props.suspend} /> ))} diff --git a/src/components/NewBranchDialog.tsx b/src/components/NewBranchDialog.tsx index cbe4a5801..bfc9c6b3c 100644 --- a/src/components/NewBranchDialog.tsx +++ b/src/components/NewBranchDialog.tsx @@ -1,11 +1,11 @@ -import * as React from 'react'; -import { classes } from 'typestyle'; -import ListItem from '@material-ui/core/ListItem'; import Dialog from '@material-ui/core/Dialog'; import DialogActions from '@material-ui/core/DialogActions'; +import ListItem from '@material-ui/core/ListItem'; import ClearIcon from '@material-ui/icons/Clear'; -import { sleep } from '../utils'; -import { Git, IGitExtension, ILogMessage } from '../tokens'; +import * as React from 'react'; +import { ListChildComponentProps, VariableSizeList } from 'react-window'; +import { classes } from 'typestyle'; +import { Logger } from '../logger'; import { actionsWrapperClass, activeListItemClass, @@ -31,9 +31,7 @@ import { titleClass, titleWrapperClass } from '../style/NewBranchDialog'; -import { SuspendModal } from './SuspendModal'; -import { Alert } from './Alert'; -import { VariableSizeList, ListChildComponentProps } from 'react-window'; +import { Git, IGitExtension, Level } from '../tokens'; const BRANCH_DESC = { current: @@ -50,6 +48,11 @@ const HEIGHT = 200; // HTML element height for the branches list * Interface describing component properties. */ export interface INewBranchDialogProps { + /** + * Extension logger + */ + logger: Logger; + /** * Git extension data model. */ @@ -60,11 +63,6 @@ export interface INewBranchDialogProps { */ open: boolean; - /** - * Boolean indicating whether to enable UI suspension. - */ - suspend: boolean; - /** * Callback to invoke upon closing the dialog. */ @@ -104,21 +102,6 @@ export interface INewBranchDialogState { * Error message. */ error: string; - - /** - * Boolean indicating whether UI interaction should be suspended (e.g., due to pending command). - */ - suspend: boolean; - - /** - * Boolean indicating whether to show an alert. - */ - alert: boolean; - - /** - * Log message. - */ - log: ILogMessage; } /** @@ -146,13 +129,7 @@ export class NewBranchDialog extends React.Component< filter: '', current: repo ? this.props.model.currentBranch.name : '', branches: repo ? this.props.model.branches : [], - error: '', - suspend: false, - alert: false, - log: { - severity: 'info', - message: '' - } + error: '' }; } @@ -169,27 +146,12 @@ export class NewBranchDialog extends React.Component< componentWillUnmount(): void { this._removeListeners(); } - - /** - * Renders the component. - * - * @returns React element - */ - render(): React.ReactElement { - return ( - - {this._renderDialog()} - {this._renderFeedback()} - - ); - } - /** * Renders a dialog for creating a new branch. * * @returns React element */ - private _renderDialog(): React.ReactElement { + render(): React.ReactElement { return ( - - - - ); - } - /** * Adds model listeners. */ @@ -427,31 +367,6 @@ export class NewBranchDialog extends React.Component< }); } - /** - * Sets the suspension state. - * - * @param bool - boolean indicating whether to suspend UI interaction - */ - private _suspend(bool: boolean): void { - if (this.props.suspend) { - this.setState({ - suspend: bool - }); - } - } - - /** - * Sets the current component log message. - * - * @param msg - log message - */ - private _log(msg: ILogMessage): void { - this.setState({ - alert: true, - log: msg - }); - } - /** * Callback invoked upon closing the dialog. * @@ -540,47 +455,30 @@ export class NewBranchDialog extends React.Component< * @returns promise which resolves upon attempting to create a new branch */ private async _createBranch(branch: string): Promise { - let result: Array; - const opts = { newBranch: true, branchname: branch }; - this._log({ - severity: 'info', + + this.props.logger.log({ + level: Level.RUNNING, message: 'Creating branch...' }); - this._suspend(true); try { - result = await Promise.all([ - sleep(1000), - this.props.model.checkout(opts) - ]); + await this.props.model.checkout(opts); } catch (err) { - this._suspend(false); this.setState({ error: err.message.replace(/^fatal:/, '') }); - this._log({ - severity: 'error', + this.props.logger.log({ + level: Level.ERROR, message: 'Failed to create branch.' }); return; } - this._suspend(false); - const res = result[1] as Git.ICheckoutResult; - if (res.code !== 0) { - this.setState({ - error: res.message - }); - this._log({ - severity: 'error', - message: 'Failed to create branch.' - }); - return; - } - this._log({ - severity: 'success', + + this.props.logger.log({ + level: Level.SUCCESS, message: 'Branch created.' }); // Close the branch dialog: @@ -594,25 +492,5 @@ export class NewBranchDialog extends React.Component< }); } - /** - * Callback invoked upon clicking on the feedback modal. - * - * @param event - event object - */ - private _onFeedbackModalClick = (): void => { - this._suspend(false); - }; - - /** - * Callback invoked upon closing a feedback alert. - * - * @param event - event object - */ - private _onFeedbackAlertClose = (): void => { - this.setState({ - alert: false - }); - }; - private _branchList: React.RefObject; } diff --git a/src/components/PastCommitNode.tsx b/src/components/PastCommitNode.tsx index d48ba4ea8..ac4546a4a 100644 --- a/src/components/PastCommitNode.tsx +++ b/src/components/PastCommitNode.tsx @@ -42,11 +42,6 @@ export interface IPastCommitNodeProps { * Jupyter App commands registry */ commands: CommandRegistry; - - /** - * Boolean indicating whether to enable UI suspension. - */ - suspend: boolean; } /** @@ -117,7 +112,6 @@ export class PastCommitNode extends React.Component< commit={this.props.commit} model={this.props.model} commands={this.props.commands} - suspend={this.props.suspend} /> )} diff --git a/src/components/ResetRevertDialog.tsx b/src/components/ResetRevertDialog.tsx index c3849d0bf..90e185cd6 100644 --- a/src/components/ResetRevertDialog.tsx +++ b/src/components/ResetRevertDialog.tsx @@ -1,28 +1,25 @@ -import * as React from 'react'; -import TextareaAutosize from 'react-textarea-autosize'; -import { classes } from 'typestyle'; -import { showErrorMessage } from '@jupyterlab/apputils'; import Dialog from '@material-ui/core/Dialog'; import DialogActions from '@material-ui/core/DialogActions'; import ClearIcon from '@material-ui/icons/Clear'; -import { sleep } from '../utils'; -import { Git, IGitExtension, ILogMessage } from '../tokens'; +import * as React from 'react'; +import TextareaAutosize from 'react-textarea-autosize'; +import { classes } from 'typestyle'; +import { Logger } from '../logger'; import { actionsWrapperClass, - commitFormClass, - commitSummaryClass, - commitDescriptionClass, buttonClass, cancelButtonClass, closeButtonClass, + commitDescriptionClass, + commitFormClass, + commitSummaryClass, contentWrapperClass, resetRevertDialogClass, submitButtonClass, titleClass, titleWrapperClass } from '../style/ResetRevertDialog'; -import { SuspendModal } from './SuspendModal'; -import { Alert } from './Alert'; +import { Git, IGitExtension, Level } from '../tokens'; /** * Interface describing component properties. @@ -44,14 +41,14 @@ export interface IResetRevertDialogProps { model: IGitExtension; /** - * Boolean indicating whether to show the dialog. + * Extension logger */ - open: boolean; + logger: Logger; /** - * Boolean indicating whether to enable UI suspension. + * Boolean indicating whether to show the dialog. */ - suspend: boolean; + open: boolean; /** * Callback invoked upon closing the dialog. @@ -77,32 +74,17 @@ export interface IResetRevertDialogState { * Boolean indicating whether component buttons should be disabled. */ disabled: boolean; - - /** - * Boolean indicating whether UI interaction should be suspended (e.g., due to pending command). - */ - suspend: boolean; - - /** - * Boolean indicating whether to show an alert. - */ - alert: boolean; - - /** - * Log message. - */ - log: ILogMessage; } /** - * React component for rendering a dialog for reseting or reverting a single commit. + * React component for rendering a dialog for resetting or reverting a single commit. */ export class ResetRevertDialog extends React.Component< IResetRevertDialogProps, IResetRevertDialogState > { /** - * Returns a React component for reseting or reverting a single commit. + * Returns a React component for resetting or reverting a single commit. * * @param props - component properties * @returns React component @@ -112,37 +94,18 @@ export class ResetRevertDialog extends React.Component< this.state = { summary: '', description: '', - disabled: false, - suspend: false, - alert: false, - log: { - severity: 'info', - message: '' - } + disabled: false }; } - /** - * Renders the component. - * - * @returns React element - */ - render(): React.ReactElement { - return ( - - {this._renderDialog()} - {this._renderFeedback()} - - ); - } - /** * Renders a dialog. * * @returns React element */ - private _renderDialog(): React.ReactElement { + render(): React.ReactElement { const shortCommit = this.props.commit.commit.slice(0, 7); + const isRevert = this.props.action === 'revert'; return (

- {this.props.action === 'revert' - ? 'Revert Changes' - : 'Reset Changes'} + {isRevert ? 'Revert Changes' : 'Reset Changes'}

{this.state.branchMenu ? ( ) : null}
); } - /** - * Renders a component to provide UI feedback. - * - * @returns React element - */ - private _renderFeedback(): React.ReactElement { - return ( - - - - - ); - } - /** * Adds model listeners. */ @@ -327,31 +281,6 @@ export class Toolbar extends React.Component { }); } - /** - * Sets the suspension state. - * - * @param bool - boolean indicating whether to suspend UI interaction - */ - private _suspend(bool: boolean): void { - if (this.props.suspend) { - this.setState({ - suspend: bool - }); - } - } - - /** - * Sets the current component log message. - * - * @param msg - log message - */ - private _log(msg: ILogMessage): void { - this.setState({ - alert: true, - log: msg - }); - } - /** * Callback invoked upon clicking a button to pull the latest changes. * @@ -359,9 +288,7 @@ export class Toolbar extends React.Component { * @returns a promise which resolves upon pulling the latest changes */ private _onPullClick = async (): Promise => { - this._suspend(true); await this.props.commands.execute(CommandIDs.gitPull); - this._suspend(false); }; /** @@ -371,9 +298,7 @@ export class Toolbar extends React.Component { * @returns a promise which resolves upon pushing the latest changes */ private _onPushClick = async (): Promise => { - this._suspend(true); await this.props.commands.execute(CommandIDs.gitPush); - this._suspend(false); }; /** @@ -407,37 +332,25 @@ export class Toolbar extends React.Component { * @returns a promise which resolves upon refreshing a repository */ private _onRefreshClick = async (): Promise => { - this._log({ - severity: 'info', + this.props.logger.log({ + level: Level.RUNNING, message: 'Refreshing...' }); - this._suspend(true); - await Promise.all([sleep(1000), this.props.refresh()]); - this._suspend(false); - this._log({ - severity: 'success', - message: 'Successfully refreshed.' - }); - }; + try { + await this.props.refresh(); - /** - * Callback invoked upon clicking on the feedback modal. - * - * @param event - event object - */ - private _onFeedbackModalClick = (): void => { - this._suspend(false); - }; - - /** - * Callback invoked upon closing a feedback alert. - * - * @param event - event object - */ - private _onFeedbackAlertClose = (): void => { - this.setState({ - alert: false - }); + this.props.logger.log({ + level: Level.SUCCESS, + message: 'Successfully refreshed.' + }); + } catch (error) { + console.error(error); + this.props.logger.log({ + level: Level.ERROR, + message: 'Failed to refresh.', + error + }); + } }; /** @@ -451,35 +364,25 @@ export class Toolbar extends React.Component { body: new GitTagDialog(this.props.model) }); if (result.button.accept) { - this._log({ - severity: 'info', + this.props.logger.log({ + level: Level.RUNNING, message: `Switching to ${result.value}...` }); - this._suspend(true); - let response: Git.ICheckoutResult; try { - response = await this.props.model.checkoutTag(result.value); - } catch (error) { - response = { - code: -1, - message: error.message || error - }; - } finally { - this._suspend(false); - } + await this.props.model.checkoutTag(result.value); - if (response.code !== 0) { - console.error(response.message); - this._log({ - severity: 'error', - message: `Fail to checkout tag ${result.value}` - }); - } else { - this._log({ - severity: 'success', + this.props.logger.log({ + level: Level.SUCCESS, message: `Switched to ${result.value}` }); + } catch (error) { + console.error(error); + this.props.logger.log({ + level: Level.ERROR, + message: `Fail to checkout tag ${result.value}`, + error + }); } } }; diff --git a/src/index.ts b/src/index.ts index 3e889ea23..a4408ebcd 100644 --- a/src/index.ts +++ b/src/index.ts @@ -164,7 +164,7 @@ async function activate( mainMenu.addMenu(createGitMenu(app.commands), { rank: 60 }); // Add a clone button to the file browser extension toolbar - addCloneButton(gitExtension, factory.defaultBrowser); + addCloneButton(gitExtension, factory.defaultBrowser, app.commands); // Add the status bar widget addStatusBarWidget(statusBar, gitExtension, settings); diff --git a/src/logger.ts b/src/logger.ts new file mode 100644 index 000000000..c12bbad31 --- /dev/null +++ b/src/logger.ts @@ -0,0 +1,40 @@ +import * as React from 'react'; +import { ISignal, Signal } from '@lumino/signaling'; +import { ILogMessage } from './tokens'; + +/** + * Logger + */ +export class Logger { + constructor() { + this._signal = new Signal(this); + } + + /** + * Signal emitted when a log message is sent + */ + get signal(): ISignal { + return this._signal; + } + + /** + * Send a log message. + * + * @param message Log message + */ + log(message: ILogMessage) { + this._signal.emit(message); + } + + private _signal: Signal; +} + +/** + * Default logger + */ +export const logger = new Logger(); + +/** + * Default logger context for React + */ +export const LoggerContext = React.createContext(logger); diff --git a/src/model.ts b/src/model.ts index 733d82e59..37041c703 100644 --- a/src/model.ts +++ b/src/model.ts @@ -1,12 +1,11 @@ import { IChangedArgs, PathExt } from '@jupyterlab/coreutils'; import { IDocumentManager } from '@jupyterlab/docmanager'; -import { ServerConnection } from '@jupyterlab/services'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; -import { LinkedList } from '@lumino/collections'; import { JSONObject } from '@lumino/coreutils'; import { Poll } from '@lumino/polling'; import { ISignal, Signal } from '@lumino/signaling'; import { requestAPI } from './git'; +import { TaskHandler } from './taskhandler'; import { Git, IGitExtension } from './tokens'; import { decodeStage } from './utils'; @@ -29,10 +28,10 @@ export class GitExtension implements IGitExtension { docmanager: IDocumentManager = null, settings?: ISettingRegistry.ISettings ) { - const self = this; this._serverRoot = serverRoot; this._docmanager = docmanager; this._settings = settings || null; + this._taskHandler = new TaskHandler(this); let interval: number; if (settings) { @@ -42,7 +41,7 @@ export class GitExtension implements IGitExtension { interval = DEFAULT_REFRESH_INTERVAL; } const poll = new Poll({ - factory: () => self.refresh(), + factory: () => this.refresh(), frequency: { interval: interval, backoff: true, @@ -200,8 +199,8 @@ export class GitExtension implements IGitExtension { /** * A signal emitted whenever a model event occurs. */ - get logger(): ISignal { - return this._logger; + get taskChanged(): ISignal { + return this._taskHandler.taskChanged; } /** @@ -220,18 +219,13 @@ export class GitExtension implements IGitExtension { */ async add(...filename: string[]): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:add:files'); - try { + await this._taskHandler.execute('git:add:files', async () => { await requestAPI('add', 'POST', { add_all: !filename, filename: filename || '', top_repo_path: path }); - } catch (err) { - throw new ServerConnection.NetworkError(err); - } finally { - this._removeTask(tid); - } + }); await this.refreshStatus(); } @@ -246,14 +240,14 @@ export class GitExtension implements IGitExtension { */ async addAllUnstaged(): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:add:files:all_unstaged'); - try { - await requestAPI('add_all_unstaged', 'POST', { - top_repo_path: path - }); - } finally { - this._removeTask(tid); - } + await this._taskHandler.execute( + 'git:add:files:all_unstaged', + async () => { + await requestAPI('add_all_unstaged', 'POST', { + top_repo_path: path + }); + } + ); await this.refreshStatus(); } @@ -268,14 +262,14 @@ export class GitExtension implements IGitExtension { */ async addAllUntracked(): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:add:files:all_untracked'); - try { - await requestAPI('add_all_untracked', 'POST', { - top_repo_path: path - }); - } finally { - this._removeTask(tid); - } + await this._taskHandler.execute( + 'git:add:files:all_untracked', + async () => { + await requestAPI('add_all_untracked', 'POST', { + top_repo_path: path + }); + } + ); await this.refreshStatus(); } @@ -292,16 +286,13 @@ export class GitExtension implements IGitExtension { */ async addRemote(url: string, name?: string): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:add:remote'); - try { + await this._taskHandler.execute('git:add:remote', async () => { await requestAPI('remote/add', 'POST', { top_repo_path: path, url, name }); - } finally { - this._removeTask(tid); - } + }); } /** @@ -320,16 +311,17 @@ export class GitExtension implements IGitExtension { */ async allHistory(count = 25): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:fetch:history'); - try { - const data = await requestAPI('all_history', 'POST', { - current_path: path, - history_count: count - }); - return data; - } finally { - this._removeTask(tid); - } + const data = await this._taskHandler.execute( + 'git:fetch:history', + async () => { + const d = await requestAPI('all_history', 'POST', { + current_path: path, + history_count: count + }); + return d; + } + ); + return data; } /** @@ -351,7 +343,6 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async checkout(options?: Git.ICheckoutOptions): Promise { - let data: Git.ICheckoutResult; const path = await this._getPathRespository(); const body = { @@ -378,24 +369,27 @@ export class GitExtension implements IGitExtension { } } - const tid = this._addTask('git:checkout'); - try { - data = await requestAPI('checkout', 'POST', body); - - if (body.checkout_branch) { - const changes = await this._changedFiles( - this._currentBranch.name, - body.branchname + const data = await this._taskHandler.execute( + 'git:checkout', + async () => { + const d = await requestAPI( + 'checkout', + 'POST', + body ); - if (changes.files) { - changes.files.forEach(file => this._revertFile(file)); + + if (body.checkout_branch) { + const changes = await this._changedFiles( + this._currentBranch.name, + body.branchname + ); + changes.files?.forEach(file => this._revertFile(file)); + } else { + this._revertFile(options.filename); } - } else { - this._revertFile(options.filename); + return d; } - } finally { - this._removeTask(tid); - } + ); if (body.checkout_branch) { await this.refreshBranch(); @@ -422,22 +416,18 @@ export class GitExtension implements IGitExtension { url: string, auth?: Git.IAuth ): Promise { - const obj: Git.IGitClone = { - current_path: path, - clone_url: url, - auth - }; - const tid = this._addTask('git:clone'); - try { - const data = await requestAPI( - 'clone', - 'POST', - obj as any - ); - return data; - } finally { - this._removeTask(tid); - } + const data = this._taskHandler.execute( + 'git:clone', + async () => { + const d = await requestAPI('clone', 'POST', { + current_path: path, + clone_url: url, + auth: auth as any + }); + return d; + } + ); + return data; } /** @@ -452,15 +442,12 @@ export class GitExtension implements IGitExtension { */ async commit(message: string): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:commit:create'); - try { + await this._taskHandler.execute('git:commit:create', async () => { await requestAPI('commit', 'POST', { commit_msg: message, top_repo_path: path }); - } finally { - this._removeTask(tid); - } + }); await this.refreshStatus(); this._headChanged.emit(); } @@ -477,20 +464,21 @@ export class GitExtension implements IGitExtension { */ async config(options?: JSONObject): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:config:' + (options ? 'set' : 'get')); - try { - if (options) { - await requestAPI('config', 'POST', { - path, - options - }); - } else { - const data = await requestAPI('config', 'POST', { path }); - return data; + const data = await this._taskHandler.execute( + 'git:config:' + (options ? 'set' : 'get'), + async () => { + if (options) { + await requestAPI('config', 'POST', { + path, + options + }); + } else { + const d = await requestAPI('config', 'POST', { path }); + return d; + } } - } finally { - this._removeTask(tid); - } + ); + return data; } /** @@ -517,20 +505,21 @@ export class GitExtension implements IGitExtension { */ async detailedLog(hash: string): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:fetch:commit_log'); - try { - const data = await requestAPI( - 'detailed_log', - 'POST', - { - selected_hash: hash, - current_path: path - } - ); - return data; - } finally { - this._removeTask(tid); - } + const data = await this._taskHandler.execute( + 'git:fetch:commit_log', + async () => { + const d = await requestAPI( + 'detailed_log', + 'POST', + { + selected_hash: hash, + current_path: path + } + ); + return d; + } + ); + return data; } /** @@ -604,14 +593,11 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async init(path: string): Promise { - const tid = this._addTask('git:init'); - try { + await this._taskHandler.execute('git:init', async () => { await requestAPI('init', 'POST', { current_path: path }); - } finally { - this._removeTask(tid); - } + }); } /** @@ -626,16 +612,17 @@ export class GitExtension implements IGitExtension { */ async log(count = 25): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:fetch:log'); - try { - const data = await requestAPI('log', 'POST', { - current_path: path, - history_count: count - }); - return data; - } finally { - this._removeTask(tid); - } + const data = this._taskHandler.execute( + 'git:fetch:log', + async () => { + const d = await requestAPI('log', 'POST', { + current_path: path, + history_count: count + }); + return d; + } + ); + return data; } /** @@ -649,21 +636,20 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async pull(auth?: Git.IAuth): Promise { - let data: Git.IPushPullResult; const path = await this._getPathRespository(); - const obj: Git.IPushPull = { - current_path: path, - auth, - cancel_on_conflict: this._settings - ? (this._settings.composite['cancelPullMergeConflict'] as boolean) - : false - }; - const tid = this._addTask('git:pull'); - try { - data = await requestAPI('pull', 'POST', obj as any); - } finally { - this._removeTask(tid); - } + const data = this._taskHandler.execute( + 'git:pull', + async () => { + const d = await requestAPI('pull', 'POST', { + current_path: path, + auth: auth as any, + cancel_on_conflict: + (this._settings?.composite['cancelPullMergeConflict'] as boolean) || + false + }); + return d; + } + ); this._headChanged.emit(); return data; } @@ -679,18 +665,17 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async push(auth?: Git.IAuth): Promise { - let data: Git.IPushPullResult; const path = await this._getPathRespository(); - const obj: Git.IPushPull = { - current_path: path, - auth - }; - const tid = this._addTask('git:push'); - try { - data = await requestAPI('push', 'POST', obj as any); - } finally { - this._removeTask(tid); - } + const data = this._taskHandler.execute( + 'git:push', + async () => { + const d = await requestAPI('push', 'POST', { + current_path: path, + auth: auth as any + }); + return d; + } + ); this._headChanged.emit(); return data; } @@ -701,13 +686,10 @@ export class GitExtension implements IGitExtension { * @returns promise which resolves upon refreshing the repository */ async refresh(): Promise { - const tid = this._addTask('git:refresh'); - try { + await this._taskHandler.execute('git:refresh', async () => { await this.refreshBranch(); await this.refreshStatus(); - } finally { - this._removeTask(tid); - } + }); } /** @@ -716,11 +698,16 @@ export class GitExtension implements IGitExtension { * @returns promise which resolves upon refreshing repository branches */ async refreshBranch(): Promise { - const tid = this._addTask('git:refresh:branches'); try { - const response = await this._branch(); - this._branches = response.branches; - this._currentBranch = response.current_branch; + const data = await this._taskHandler.execute( + 'git:refresh:branches', + async () => { + const response = await this._branch(); + return response; + } + ); + this._branches = data.branches; + this._currentBranch = data.current_branch; if (this._currentBranch) { // Set up the marker obj for the current (valid) repo/branch combination this._setMarker(this.pathRepository, this._currentBranch.name); @@ -731,8 +718,6 @@ export class GitExtension implements IGitExtension { if (!(error instanceof Git.NotInRepository)) { throw error; } - } finally { - this._removeTask(tid); } } @@ -742,8 +727,6 @@ export class GitExtension implements IGitExtension { * @returns promise which resolves upon refreshing the repository status */ async refreshStatus(): Promise { - let data: Git.IStatusResult; - let path: string; try { path = await this._getPathRespository(); @@ -755,24 +738,28 @@ export class GitExtension implements IGitExtension { return; } - const tid = this._addTask('git:refresh:status'); try { - data = await requestAPI('status', 'POST', { - current_path: path - }); + const data = await this._taskHandler.execute( + 'git:refresh:status', + async () => { + const d = await requestAPI('status', 'POST', { + current_path: path + }); + return d; + } + ); + + this._setStatus( + data.files.map(file => { + return { ...file, status: decodeStage(file.x, file.y) }; + }) + ); } catch (err) { // TODO we should notify the user this._setStatus([]); console.error(err); return; - } finally { - this._removeTask(tid); } - this._setStatus( - data.files.map(file => { - return { ...file, status: decodeStage(file.x, file.y) }; - }) - ); } /** @@ -791,31 +778,24 @@ export class GitExtension implements IGitExtension { */ async reset(filename?: string): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:reset:changes'); - const reset_all = filename === undefined; - let files; - if (reset_all) { - files = (await this._changedFiles('INDEX', 'HEAD'))['files']; - } - try { + await this._taskHandler.execute('git:reset:changes', async () => { + const reset_all = filename === undefined; + let files: string[]; + if (reset_all) { + files = (await this._changedFiles('INDEX', 'HEAD')).files; + } else { + files = [filename]; + } await requestAPI('reset', 'POST', { reset_all: filename === undefined, filename: filename === undefined ? null : filename, top_repo_path: path }); - if (reset_all) { - if (files) { - files.forEach(file => { - this._revertFile(file); - }); - } - } else { - this._revertFile(filename); - } - } finally { - this._removeTask(tid); - } + files.forEach(file => { + this._revertFile(file); + }); + }); await this.refreshStatus(); } @@ -835,21 +815,18 @@ export class GitExtension implements IGitExtension { */ async resetToCommit(hash = ''): Promise { const path = await this._getPathRespository(); - const files = (await this._changedFiles(null, null, hash))['files']; - const tid = this._addTask('git:reset:hard'); - try { + await this._taskHandler.execute('git:reset:hard', async () => { + const files = (await this._changedFiles(null, null, hash)).files; + await requestAPI('reset_to_commit', 'POST', { commit_id: hash, top_repo_path: path }); - if (files) { - files.forEach(file => { - this._revertFile(file); - }); - } - } finally { - this._removeTask(tid); - } + + files?.forEach(file => { + this._revertFile(file); + }); + }); await this.refreshBranch(); this._headChanged.emit(); } @@ -864,19 +841,20 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async showPrefix(path: string): Promise { - const tid = this._addTask('git:fetch:prefix_path'); - try { - const data = await requestAPI( - 'show_prefix', - 'POST', - { - current_path: path - } - ); - return data; - } finally { - this._removeTask(tid); - } + const data = await this._taskHandler.execute( + 'git:fetch:prefix_path', + async () => { + const d = await requestAPI( + 'show_prefix', + 'POST', + { + current_path: path + } + ); + return d; + } + ); + return data; } /** @@ -889,19 +867,20 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async showTopLevel(path: string): Promise { - const tid = this._addTask('git:fetch:top_level_path'); - try { - const data = await requestAPI( - 'show_top_level', - 'POST', - { - current_path: path - } - ); - return data; - } finally { - this._removeTask(tid); - } + const data = this._taskHandler.execute( + 'git:fetch:top_level_path', + async () => { + const d = await requestAPI( + 'show_top_level', + 'POST', + { + current_path: path + } + ); + return d; + } + ); + return data; } /** @@ -915,15 +894,16 @@ export class GitExtension implements IGitExtension { */ async tags(): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:tag:list'); - try { - const data = await requestAPI('tags', 'POST', { - current_path: path - }); - return data; - } finally { - this._removeTask(tid); - } + const data = await this._taskHandler.execute( + 'git:tag:list', + async () => { + const d = await requestAPI('tags', 'POST', { + current_path: path + }); + return d; + } + ); + return data; } /** @@ -938,20 +918,21 @@ export class GitExtension implements IGitExtension { */ async checkoutTag(tag: string): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:tag:checkout'); - try { - const data = await requestAPI( - 'tag_checkout', - 'POST', - { - current_path: path, - tag_id: tag - } - ); - return data; - } finally { - this._removeTask(tid); - } + const data = await this._taskHandler.execute( + 'git:tag:checkout', + async () => { + const d = await requestAPI( + 'tag_checkout', + 'POST', + { + current_path: path, + tag_id: tag + } + ); + return d; + } + ); + return data; } /** @@ -1008,21 +989,18 @@ export class GitExtension implements IGitExtension { */ async revertCommit(message: string, hash: string): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:commit:revert'); - const files = (await this._changedFiles(null, null, hash + '^!'))['files']; - try { + await this._taskHandler.execute('git:commit:revert', async () => { + const files = (await this._changedFiles(null, null, hash + '^!')).files; + await requestAPI('delete_commit', 'POST', { commit_id: hash, top_repo_path: path }); - if (files) { - files.forEach(file => { - this._revertFile(file); - }); - } - } finally { - this._removeTask(tid); - } + + files?.forEach(file => { + this._revertFile(file); + }); + }); await this.commit(message); } @@ -1038,15 +1016,16 @@ export class GitExtension implements IGitExtension { */ protected async _branch(): Promise { const path = await this._getPathRespository(); - const tid = this._addTask('git:fetch:branches'); - try { - const data = await requestAPI('branch', 'POST', { - current_path: path - }); - return data; - } finally { - this._removeTask(tid); - } + const data = await this._taskHandler.execute( + 'git:fetch:branches', + async () => { + const d = await requestAPI('branch', 'POST', { + current_path: path + }); + return d; + } + ); + return data; } /** @@ -1107,40 +1086,6 @@ export class GitExtension implements IGitExtension { this._statusChanged.emit(this._status); } - /** - * Adds a task to the list of pending model tasks. - * - * @param task - task name - * @returns task identifier - */ - private _addTask(task: string): number { - // Generate a unique task identifier: - const id = this._generateTaskID(); - - // Add the task to our list of pending tasks: - this._taskList.addLast({ - id: id, - task: task - }); - - // If this task is the only task, broadcast the task... - if (this._taskList.length === 1) { - this._logger.emit(task); - } - // Return the task identifier to allow consumers to remove the task once completed: - return id; - } - - /** - * Generates a unique task identifier. - * - * @returns task identifier - */ - private _generateTaskID(): number { - this._taskID += 1; - return this._taskID; - } - /** * open new editor or show an existing editor of the * .gitignore file. If the editor does not have unsaved changes @@ -1157,35 +1102,6 @@ export class GitExtension implements IGitExtension { } } - /** - * Removes a task from the list of pending model tasks. - * - * @param id - task identifier - */ - private _removeTask(task: number): void { - let node = this._taskList.firstNode; - - // Check the first node... - if (node && node.value.id === task) { - this._taskList.removeNode(node); - } else { - // Walk the task list looking for a task with the provided identifier... - while (node.next) { - node = node.next; - if (node.value && node.value.id === task) { - this._taskList.removeNode(node); - break; - } - } - } - // Check for pending tasks and broadcast the oldest pending task... - if (this._taskList.length === 0) { - this._logger.emit('git:idle'); - } else { - this._logger.emit(this._taskList.first.task); - } - } - /** * if file is open in JupyterLab find the widget and ensure the JupyterLab * version matches the version on disk. Do nothing if the file has unsaved changes @@ -1222,9 +1138,9 @@ export class GitExtension implements IGitExtension { private _readyPromise: Promise = Promise.resolve(); private _pendingReadyPromise = 0; private _poll: Poll; - private _taskList: LinkedList = new LinkedList(); - private _taskID = 0; private _settings: ISettingRegistry.ISettings | null; + private _taskHandler: TaskHandler; + private _headChanged = new Signal(this); private _markChanged = new Signal(this); private _repositoryChanged = new Signal< @@ -1232,7 +1148,6 @@ export class GitExtension implements IGitExtension { IChangedArgs >(this); private _statusChanged = new Signal(this); - private _logger = new Signal(this); } export class BranchMarker implements Git.IBranchMarker { diff --git a/src/taskhandler.ts b/src/taskhandler.ts new file mode 100644 index 000000000..c57012f4a --- /dev/null +++ b/src/taskhandler.ts @@ -0,0 +1,111 @@ +import { LinkedList } from '@lumino/collections'; +import { UUID } from '@lumino/coreutils'; +import { ISignal, Signal } from '@lumino/signaling'; + +/** + * A generic task handler + */ +export class TaskHandler { + constructor(model: T) { + this._taskChanged = new Signal(model); + } + + /** + * Signal emitted when a task starts + * + * 'empty' is emitted each time the task list have processed all tasks + */ + get taskChanged(): ISignal { + return this._taskChanged; + } + + /** + * Adds a task to the list of pending model tasks. + * + * #Note: + * This will add a task name in the queue but the task + * execution remains in the hand of the caller. + * In particular it is the responsibility of the caller + * to call `remove(taskID)` when the task is executed. + * + * @param task - task name + * @returns task identifier + */ + add(task: string): string { + // Generate a unique task identifier: + const id = this._generateTaskID(); + + // Add the task to our list of pending tasks: + this._taskList.addLast({ + id: id, + task: task + }); + + // If this task is the only task, broadcast the task... + if (this._taskList.length === 1) { + this._taskChanged.emit(task); + } + // Return the task identifier to allow consumers to remove the task once completed: + return id; + } + + /** + * Add a asynchronous task to the stack and execute it + * + * @param name Name of the task + * @param callable Asynchronous task to be executed + * + * @returns The result of the task + */ + async execute(name: string, callable: () => Promise): Promise { + const taskID = this.add(name); + try { + const result = await callable(); + return result; + } finally { + this.remove(taskID); + } + } + + /** + * Removes a task from the list of pending model tasks. + * + * @param id - task identifier + */ + remove(taskID: string): void { + let node = this._taskList.firstNode; + + // Check the first node... + if (node?.value.id === taskID) { + this._taskList.removeNode(node); + } else { + // Walk the task list looking for a task with the provided identifier... + while (node.next) { + node = node.next; + if (node.value && node.value.id === taskID) { + this._taskList.removeNode(node); + break; + } + } + } + + // Check for pending tasks and broadcast the oldest pending task... + if (this._taskList.length === 0) { + this._taskChanged.emit('empty'); + } else { + this._taskChanged.emit(this._taskList.first.task); + } + } + + /** + * Generates a unique task identifier. + * + * @returns task identifier + */ + private _generateTaskID(): string { + return UUID.uuid4(); + } + + private _taskChanged: Signal; + private _taskList: LinkedList = new LinkedList(); +} diff --git a/src/tokens.ts b/src/tokens.ts index 18b3e0ccf..bfd8accf2 100644 --- a/src/tokens.ts +++ b/src/tokens.ts @@ -26,11 +26,6 @@ export interface IGitExtension extends IDisposable { */ readonly headChanged: ISignal; - /** - * A signal emitted whenever a model event occurs. - */ - readonly logger: ISignal; - /** * Top level path of the current Git repository */ @@ -63,6 +58,11 @@ export interface IGitExtension extends IDisposable { */ readonly statusChanged: ISignal; + /** + * A signal emitted whenever a model task event occurs. + */ + readonly taskChanged: ISignal; + /** * Add one or more files to the repository staging area. * @@ -740,16 +740,27 @@ export namespace Git { /** * Log message severity. */ -export type Severity = 'error' | 'warning' | 'info' | 'success'; +export enum Level { + SUCCESS = 10, + INFO = 20, + RUNNING = 30, + WARNING = 40, + ERROR = 50 +} /** * Interface describing a component log message. */ export interface ILogMessage { /** - * Message severity. + * Error object. + */ + error?: Error; + + /** + * Message level. */ - severity: Severity; + level: Level; /** * Message text. diff --git a/src/widgets/GitCloneForm.ts b/src/widgets/GitCloneForm.ts new file mode 100644 index 000000000..badd923f8 --- /dev/null +++ b/src/widgets/GitCloneForm.ts @@ -0,0 +1,36 @@ +import { Widget } from '@lumino/widgets'; + +/** + * The UI for the form fields shown within the Clone modal. + */ +export class GitCloneForm extends Widget { + /** + * Create a redirect form. + */ + constructor() { + super({ node: GitCloneForm.createFormNode() }); + } + + /** + * Returns the input value. + */ + getValue(): string { + return encodeURIComponent(this.node.querySelector('input').value); + } + + private static createFormNode(): HTMLElement { + const node = document.createElement('div'); + const label = document.createElement('label'); + const input = document.createElement('input'); + const text = document.createElement('span'); + + node.className = 'jp-RedirectForm'; + text.textContent = 'Enter the Clone URI of the repository'; + input.placeholder = 'https://host.com/org/repo.git'; + + label.appendChild(text); + label.appendChild(input); + node.appendChild(label); + return node; + } +} diff --git a/src/widgets/GitWidget.tsx b/src/widgets/GitWidget.tsx index 1fca40e8c..bd8b4cbe7 100644 --- a/src/widgets/GitWidget.tsx +++ b/src/widgets/GitWidget.tsx @@ -1,12 +1,15 @@ -import { ReactWidget } from '@jupyterlab/apputils'; +import { ReactWidget, UseSignal } from '@jupyterlab/apputils'; import { FileBrowserModel } from '@jupyterlab/filebrowser'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; import { CommandRegistry } from '@lumino/commands'; import { Widget } from '@lumino/widgets'; import * as React from 'react'; +import { Feedback } from '../components/Feedback'; import { GitPanel } from '../components/GitPanel'; +import { LoggerContext } from '../logger'; import { GitExtension } from '../model'; import { gitWidgetStyle } from '../style/GitWidgetStyle'; +import { ILogMessage, Level } from '../tokens'; /** * A class that exposes the git plugin Widget. @@ -23,25 +26,42 @@ export class GitWidget extends ReactWidget { this.node.id = 'GitSession-root'; this.addClass(gitWidgetStyle); - this._model = model; this._commands = commands; - this._settings = settings; this._filebrowser = filebrowser; + this._model = model; + this._settings = settings; } render() { return ( - + + {logger => ( + + + + {(sender, log) => + log?.message ? ( + + ) : null + } + + + )} + ); } - private _model: GitExtension; private _commands: CommandRegistry; - private _settings: ISettingRegistry.ISettings; private _filebrowser: FileBrowserModel; + private _model: GitExtension; + private _settings: ISettingRegistry.ISettings; } diff --git a/src/widgets/StatusWidget.ts b/src/widgets/StatusWidget.ts index 65511fbf4..8eea7a122 100644 --- a/src/widgets/StatusWidget.ts +++ b/src/widgets/StatusWidget.ts @@ -75,7 +75,7 @@ export function addStatusBarWidget( isActive: Private.isStatusWidgetActive(settings), activeStateChanged: settings && settings.changed }); - model.logger.connect(Private.createEventCallback(statusWidget)); + model.taskChanged.connect(Private.createEventCallback(statusWidget)); } /* eslint-disable no-inner-declarations */ namespace Private { @@ -99,6 +99,9 @@ namespace Private { function onEvent(model: IGitExtension, event: string) { let status; switch (event) { + case 'empty': + status = 'idle'; + break; case 'git:checkout': status = 'checking out...'; break; @@ -111,9 +114,6 @@ namespace Private { case 'git:commit:revert': status = 'reverting changes...'; break; - case 'git:idle': - status = 'idle'; - break; case 'git:init': status = 'initializing repository...'; break; diff --git a/src/widgets/gitClone.tsx b/src/widgets/gitClone.tsx index 90dc6dc5e..cea3653e1 100644 --- a/src/widgets/gitClone.tsx +++ b/src/widgets/gitClone.tsx @@ -1,21 +1,21 @@ import { - Dialog, ReactWidget, - showDialog, - showErrorMessage, ToolbarButtonComponent, UseSignal } from '@jupyterlab/apputils'; import { IChangedArgs } from '@jupyterlab/coreutils'; import { FileBrowser } from '@jupyterlab/filebrowser'; -import { Widget } from '@lumino/widgets'; +import { CommandRegistry } from '@lumino/commands'; import * as React from 'react'; -import { AUTH_ERROR_MESSAGES } from '../git'; +import { CommandIDs } from '../commandsAndMenu'; import { cloneIcon } from '../style/icons'; import { IGitExtension } from '../tokens'; -import { GitCredentialsForm } from './CredentialsBox'; -export function addCloneButton(model: IGitExtension, filebrowser: FileBrowser) { +export function addCloneButton( + model: IGitExtension, + filebrowser: FileBrowser, + commands: CommandRegistry +) { filebrowser.toolbar.addItem( 'gitClone', ReactWidget.create( @@ -31,9 +31,8 @@ export function addCloneButton(model: IGitExtension, filebrowser: FileBrowser) { { - await doGitClone(model, filebrowser.model.path); - filebrowser.model.refresh(); + onClick={() => { + commands.execute(CommandIDs.gitClone); }} tooltip={'Git Clone'} /> @@ -42,120 +41,3 @@ export function addCloneButton(model: IGitExtension, filebrowser: FileBrowser) { ) ); } - -/** - * Makes the API call to the server. - * - * @param cloneUrl - */ -async function makeApiCall( - model: IGitExtension, - path: string, - cloneUrl: string -) { - try { - let response = await model.clone(path, cloneUrl); - - let retry = false; - while (response.code !== 0) { - if ( - response.code === 128 && - AUTH_ERROR_MESSAGES.map( - message => response.message.indexOf(message) > -1 - ).indexOf(true) > -1 - ) { - // request user credentials and try to clone again - const result = await showDialog({ - title: 'Git credentials required', - body: new GitCredentialsForm( - 'Enter credentials for remote repository', - retry ? 'Incorrect username or password.' : '' - ) - }); - retry = true; - - if (result.button.accept) { - // user accepted attempt to login - // try to clone again - response = await model.clone(path, cloneUrl, result.value); - } else { - showErrorMessage('Clone failed', response.message, [ - Dialog.warnButton({ label: 'DISMISS' }) - ]); - break; - } - } else { - showErrorMessage('Clone failed', response.message, [ - Dialog.warnButton({ label: 'DISMISS' }) - ]); - break; - } - } - } catch (error) { - showErrorMessage('Clone failed', error, [ - Dialog.warnButton({ label: 'DISMISS' }) - ]); - } -} - -/** - * Callback method on Git Clone button in the File Browser toolbar. - * 1. Invokes a new dialog box with form fields. - * 2. Invokes the server API with the form input. - */ -export async function doGitClone( - model: IGitExtension, - path: string -): Promise { - const result = await showDialog({ - title: 'Clone a repo', - body: new GitCloneForm(), - focusNodeSelector: 'input', - buttons: [Dialog.cancelButton(), Dialog.okButton({ label: 'CLONE' })] - }); - - if (result.button.accept) { - if (typeof result.value !== 'undefined' && result.value) { - const cloneUrl: string = result.value; - await makeApiCall(model, path, cloneUrl); - } - } -} - -/** - * The UI for the form fields shown within the Clone modal. - */ -class GitCloneForm extends Widget { - /** - * Create a redirect form. - */ - constructor() { - super({ node: GitCloneForm.createFormNode() }); - } - - private static createFormNode(): HTMLElement { - const node = document.createElement('div'); - const label = document.createElement('label'); - const input = document.createElement('input'); - const text = document.createElement('span'); - const warning = document.createElement('div'); - - node.className = 'jp-RedirectForm'; - warning.className = 'jp-RedirectForm-warning'; - text.textContent = 'Enter the Clone URI of the repository'; - input.placeholder = 'https://host.com/org/repo.git'; - - label.appendChild(text); - label.appendChild(input); - node.appendChild(label); - node.appendChild(warning); - return node; - } - - /** - * Returns the input value. - */ - getValue(): string { - return encodeURIComponent(this.node.querySelector('input').value); - } -} diff --git a/src/widgets/gitPushPull.ts b/src/widgets/gitPushPull.ts deleted file mode 100644 index 77c6db8a0..000000000 --- a/src/widgets/gitPushPull.ts +++ /dev/null @@ -1,119 +0,0 @@ -import { Spinner } from '@jupyterlab/apputils'; -import { Widget } from '@lumino/widgets'; -import { AUTH_ERROR_MESSAGES } from '../git'; -import { Git, IGitExtension } from '../tokens'; - -export enum Operation { - Pull = 'Pull', - Push = 'Push' -} - -/** - * The UI for the content shown within the Git push/pull modal. - */ -export class GitPullPushDialog extends Widget { - private _spinner: Spinner; - private _model: IGitExtension; - private _body: HTMLElement; - private _operation: Operation; - - /** - * Instantiates the dialog and makes the relevant service API call. - */ - constructor(model: IGitExtension, operation: Operation, auth?: Git.IAuth) { - super(); - this._model = model; - this._operation = operation; - - this._body = this.createBody(); - this.node.appendChild(this._body); - - this._spinner = new Spinner(); - this.node.appendChild(this._spinner.node); - - this.executeGitApi(auth); - } - - /** - * Executes the relevant service API depending on the _operation and handles response and errors. - * @param currentFileBrowserPath the path to the current repo - */ - private executeGitApi(auth?: Git.IAuth) { - switch (this._operation) { - case Operation.Pull: - this._model - .pull(auth) - .then(response => { - this.handleResponse(response); - }) - .catch(error => this.handleError(error.message)); - break; - case Operation.Push: - this._model - .push(auth) - .then(response => { - this.handleResponse(response); - }) - .catch(error => this.handleError(error.message)); - break; - default: - throw new Error(`Invalid _operation type: ${this._operation}`); - } - } - - /** - * Handles the response from the server by removing the _spinner and showing the appropriate - * success or error message. - * @param response the response from the server API call - */ - private async handleResponse(response: Git.IPushPullResult) { - if (response.code !== 0) { - this.handleError(response.message); - } else { - this.handleSuccess(); - } - } - - private handleError( - message = 'Unexpected failure. Please check your Jupyter server logs for more details.' - ): void { - if ( - AUTH_ERROR_MESSAGES.map( - errorMessage => message.indexOf(errorMessage) > -1 - ).indexOf(true) > -1 - ) { - this.parent!.parent!.close(); // eslint-disable-line @typescript-eslint/no-non-null-assertion - } - - this.node.removeChild(this._spinner.node); - this._spinner.dispose(); - const label = document.createElement('label'); - const text = document.createElement('span'); - text.textContent = `Git ${this._operation} failed with error:`; - const errorMessage = document.createElement('span'); - errorMessage.textContent = message; - errorMessage.setAttribute( - 'style', - 'background-color:var(--jp-rendermime-error-background)' - ); - label.appendChild(text); - label.appendChild(document.createElement('p')); - label.appendChild(errorMessage); - this._body.appendChild(label); - } - - private handleSuccess(): void { - this.node.removeChild(this._spinner.node); - this._spinner.dispose(); - const label = document.createElement('label'); - const text = document.createElement('span'); - text.textContent = `Git ${this._operation} completed successfully`; - label.appendChild(text); - this._body.appendChild(label); - } - private createBody(): HTMLElement { - const node = document.createElement('div'); - node.className = 'jp-RedirectForm'; - return node; - } -} diff --git a/tests/test-components/BranchMenu.spec.tsx b/tests/test-components/BranchMenu.spec.tsx index d11876eb4..5b6a166ef 100644 --- a/tests/test-components/BranchMenu.spec.tsx +++ b/tests/test-components/BranchMenu.spec.tsx @@ -3,6 +3,7 @@ import 'jest'; import * as React from 'react'; import { BranchMenu } from '../../src/components/BranchMenu'; import * as git from '../../src/git'; +import { Logger } from '../../src/logger'; import { GitExtension } from '../../src/model'; import { listItemClass } from '../../src/style/BranchMenu'; import { mockedRequestAPI, defaultMockedResponses } from '../utils'; @@ -83,7 +84,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const menu = new BranchMenu(props); expect(menu).toBeInstanceOf(BranchMenu); @@ -93,7 +94,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const menu = new BranchMenu(props); expect(menu.state.filter).toEqual(''); @@ -103,7 +104,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const menu = new BranchMenu(props); expect(menu.state.branchDialog).toEqual(false); @@ -115,7 +116,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); const node = component.find('input[type="text"]').first(); @@ -126,7 +127,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); const node = component.find('input[type="text"]').first(); @@ -137,7 +138,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); component.setState({ @@ -151,7 +152,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); component.setState({ @@ -168,7 +169,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); const node = component.find('input[type="button"]').first(); @@ -179,7 +180,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); const node = component.find('input[type="button"]').first(); @@ -190,7 +191,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = render(); const nodes = component.find(`.${listItemClass}`); @@ -210,7 +211,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = render(); const nodes = component.find(`.${listItemClass}`); @@ -227,7 +228,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); const node = component.find('NewBranchDialog').first(); @@ -238,7 +239,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); component.setState({ @@ -256,7 +257,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = mount(); const nodes = component.find(`.${listItemClass}[title*="${BRANCHES[1].name}"]`); @@ -272,7 +273,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: true, - suspend: false + logger: new Logger(), }; const component = mount(); const nodes = component.find(`.${listItemClass}[title*="${BRANCHES[1].name}"]`); @@ -294,7 +295,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: false, - suspend: false + logger: new Logger(), }; const component = shallow(); @@ -312,7 +313,7 @@ describe('BranchMenu', () => { const props = { model: model, branching: true, - suspend: false + logger: new Logger(), }; const component = shallow(); diff --git a/tests/test-components/GitPanel.spec.tsx b/tests/test-components/GitPanel.spec.tsx index 25973e2f8..c9decde82 100644 --- a/tests/test-components/GitPanel.spec.tsx +++ b/tests/test-components/GitPanel.spec.tsx @@ -3,6 +3,7 @@ import { JSONObject } from '@lumino/coreutils'; import 'jest'; import { GitPanel, IGitPanelProps } from '../../src/components/GitPanel'; import * as git from '../../src/git'; +import { Logger } from '../../src/logger'; import { GitExtension as GitModel } from '../../src/model'; import { defaultMockedResponses, @@ -46,6 +47,7 @@ describe('GitPanel', () => { const props: IGitPanelProps = { model: null, commands: null, + logger: new Logger(), settings: null, filebrowser: null }; diff --git a/tests/test-components/HistorySideBar.spec.tsx b/tests/test-components/HistorySideBar.spec.tsx index 4cca9d05d..7c1fb25f0 100644 --- a/tests/test-components/HistorySideBar.spec.tsx +++ b/tests/test-components/HistorySideBar.spec.tsx @@ -21,8 +21,7 @@ describe('HistorySideBar', () => { ], branches: [], model: null, - commands: null, - suspend: false + commands: null }; test('renders commit nodes', () => { const historySideBar = shallow(); diff --git a/tests/test-components/PastCommitNode.spec.tsx b/tests/test-components/PastCommitNode.spec.tsx index 70296fd79..7fb99b04d 100644 --- a/tests/test-components/PastCommitNode.spec.tsx +++ b/tests/test-components/PastCommitNode.spec.tsx @@ -56,8 +56,7 @@ describe('PastCommitNode', () => { pre_commit: 'pre_commit' }, branches: branches, - commands: null, - suspend: false + commands: null }; test('Includes commit info', () => { diff --git a/tests/test-components/Toolbar.spec.tsx b/tests/test-components/Toolbar.spec.tsx index 3c73e5fc0..328468b70 100644 --- a/tests/test-components/Toolbar.spec.tsx +++ b/tests/test-components/Toolbar.spec.tsx @@ -6,6 +6,7 @@ import { CommandIDs } from '../../src/commandsAndMenu'; import { ActionButton } from '../../src/components/ActionButton'; import { Toolbar } from '../../src/components/Toolbar'; import * as git from '../../src/git'; +import { Logger } from '../../src/logger'; import { GitExtension } from '../../src/model'; import { pullIcon, pushIcon } from '../../src/style/icons'; import { toolbarMenuButtonClass } from '../../src/style/Toolbar'; @@ -47,7 +48,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -61,7 +62,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -75,7 +76,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -91,7 +92,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -108,7 +109,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -127,7 +128,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -144,7 +145,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -163,7 +164,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -181,7 +182,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -202,7 +203,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -219,7 +220,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -236,7 +237,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -253,7 +254,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -273,7 +274,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -289,7 +290,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: jest.fn() @@ -310,7 +311,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: mockedExecute @@ -335,7 +336,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: async () => {}, commands: { execute: mockedExecute @@ -360,7 +361,7 @@ describe('Toolbar', () => { const props = { model: model, branching: false, - suspend: false, + logger: new Logger(), refresh: spy, commands: { execute: jest.fn()