From 34ca19e4df8305d678f3f82827f62d0c456e9e2f Mon Sep 17 00:00:00 2001 From: Marc Udoff Date: Tue, 21 Jul 2020 16:37:39 -0400 Subject: [PATCH 1/5] Clean up git menu --- src/gitMenuCommands.ts | 11 +++++++---- src/index.ts | 22 ++++++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/gitMenuCommands.ts b/src/gitMenuCommands.ts index bfafa0dee..b40b23afb 100644 --- a/src/gitMenuCommands.ts +++ b/src/gitMenuCommands.ts @@ -63,7 +63,8 @@ export function addCommands( console.error(e); main.dispose(); } - } + }, + isVisible: () => model.pathRepository !== null }); /** Add open/go to git interface command */ @@ -95,7 +96,8 @@ export function addCommands( await model.init(currentPath); model.pathRepository = currentPath; } - } + }, + isVisible: () => model.pathRepository === null }); /** Open URL externally */ @@ -129,7 +131,7 @@ export function addCommands( commands.addCommand(CommandIDs.gitAddRemote, { label: 'Add remote repository', caption: 'Add a Git remote repository', - isEnabled: () => model.pathRepository !== null, + isVisible: () => model.pathRepository !== null, execute: async args => { if (model.pathRepository === null) { console.warn('Not in a Git repository. Unable to add a remote.'); @@ -168,6 +170,7 @@ export function addCommands( execute: async () => { await doGitClone(model, fileBrowser.model.path); fileBrowser.model.refresh(); - } + }, + isVisible: () => model.pathRepository === null }); } diff --git a/src/index.ts b/src/index.ts index ee657cae8..0054b0a5d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -143,30 +143,32 @@ function createGitMenu( const menu = new Menu({ commands }); menu.title.label = 'Git'; [ - CommandIDs.gitUI, - CommandIDs.gitTerminalCommand, CommandIDs.gitInit, CommandIDs.gitClone, - CommandIDs.gitAddRemote + CommandIDs.gitAddRemote, + CommandIDs.gitTerminalCommand ].forEach(command => { menu.addItem({ command }); }); + menu.addItem({ type: 'separator' }); + + menu.addItem({ command: CommandIDs.gitToggleSimpleStaging }); + + menu.addItem({ command: CommandIDs.gitToggleDoubleClickDiff }); + + menu.addItem({ type: 'separator' }); + const tutorial = new Menu({ commands }); - tutorial.title.label = ' Tutorial '; + tutorial.title.label = ' Help '; RESOURCES.map(args => { tutorial.addItem({ args, command: CommandIDs.gitOpenUrl }); }); - menu.addItem({ type: 'submenu', submenu: tutorial }); - - menu.addItem({ type: 'separator' }); - menu.addItem({ command: CommandIDs.gitToggleSimpleStaging }); - - menu.addItem({ command: CommandIDs.gitToggleDoubleClickDiff }); + menu.addItem({ type: 'submenu', submenu: tutorial }); return menu; } From b78137cc4441f9d319e06f186a666fbda7ec2b6c Mon Sep 17 00:00:00 2001 From: Marc Udoff Date: Thu, 23 Jul 2020 12:05:54 -0400 Subject: [PATCH 2/5] Fix review comments --- src/components/Toolbar.tsx | 64 +++++------------------------- src/gitMenuCommands.ts | 80 ++++++++++++++++++++++++++++++++++++-- src/index.ts | 2 + src/tokens.ts | 3 ++ 4 files changed, 91 insertions(+), 58 deletions(-) diff --git a/src/components/Toolbar.tsx b/src/components/Toolbar.tsx index 791272798..eae5e6873 100644 --- a/src/components/Toolbar.tsx +++ b/src/components/Toolbar.tsx @@ -1,4 +1,3 @@ -import { Dialog, showDialog } from '@jupyterlab/apputils'; import { PathExt } from '@jupyterlab/coreutils'; import { caretDownIcon, @@ -22,52 +21,9 @@ import { toolbarNavClass } from '../style/Toolbar'; import { IGitExtension } from '../tokens'; -import { GitCredentialsForm } from '../widgets/CredentialsBox'; -import { GitPullPushDialog, Operation } from '../widgets/gitPushPull'; import { ActionButton } from './ActionButton'; import { BranchMenu } from './BranchMenu'; - -/** - * Displays an error dialog when a Git operation fails. - * - * @private - * @param model - Git extension model - * @param operation - Git operation name - * @returns Promise for displaying a dialog - */ -async function showGitOperationDialog( - model: IGitExtension, - operation: Operation -): 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; - } - - result = await showDialog({ - title: title, - body: new GitPullPushDialog(model, operation, credentials.value), - buttons: [Dialog.okButton({ label: 'DISMISS' })] - }); - retry = true; - } -} +import { CommandIDs } from '../gitMenuCommands'; /** * Interface describing component properties. @@ -304,11 +260,10 @@ export class Toolbar extends React.Component { * @param event - event object */ private _onPullClick = (): void => { - showGitOperationDialog(this.props.model, Operation.Pull).catch(reason => { - console.error( - `Encountered an error when pulling changes. Error: ${reason}` - ); - }); + const commands = this.props.model.commands; + if (commands) { + commands.execute(CommandIDs.gitPull); + } }; /** @@ -317,11 +272,10 @@ export class Toolbar extends React.Component { * @param event - event object */ private _onPushClick = (): void => { - showGitOperationDialog(this.props.model, Operation.Push).catch(reason => { - console.error( - `Encountered an error when pushing changes. Error: ${reason}` - ); - }); + const commands = this.props.model.commands; + if (commands) { + commands.execute(CommandIDs.gitPush); + } }; /** diff --git a/src/gitMenuCommands.ts b/src/gitMenuCommands.ts index b40b23afb..ef1e6c6d9 100644 --- a/src/gitMenuCommands.ts +++ b/src/gitMenuCommands.ts @@ -11,6 +11,8 @@ import { ISettingRegistry } from '@jupyterlab/settingregistry'; import { ITerminal } from '@jupyterlab/terminal'; import { IGitExtension } from './tokens'; import { doGitClone } from './widgets/gitClone'; +import { GitPullPushDialog, Operation } from './widgets/gitPushPull'; +import { GitCredentialsForm } from './widgets/CredentialsBox'; /** * The command IDs used by the git plugin. @@ -24,6 +26,8 @@ export namespace CommandIDs { export const gitToggleDoubleClickDiff = 'git:toggle-double-click-diff'; export const gitAddRemote = 'git:add-remote'; export const gitClone = 'git:clone'; + export const gitPush = 'git:push'; + export const gitPull = 'git:pull'; } /** @@ -82,8 +86,8 @@ export function addCommands( /** Add git init command */ commands.addCommand(CommandIDs.gitInit, { - label: 'Init', - caption: ' Create an empty Git repository or reinitialize an existing one', + label: 'Initialize a Repository', + caption: 'Create an empty Git repository or reinitialize an existing one', execute: async () => { const currentPath = fileBrowser.model.path; const result = await showDialog({ @@ -164,7 +168,7 @@ export function addCommands( /** Add git clone command */ commands.addCommand(CommandIDs.gitClone, { - label: 'Clone', + label: 'Clone a Repository', caption: 'Clone a repository from a URL', isEnabled: () => model.pathRepository === null, execute: async () => { @@ -173,4 +177,74 @@ export function addCommands( }, isVisible: () => model.pathRepository === null }); + + /** + * Displays an error dialog when a Git operation fails. + * + * @private + * @param model - Git extension model + * @param operation - Git operation name + * @returns Promise for displaying a dialog + */ + async function showGitOperationDialog( + model: IGitExtension, + operation: Operation + ): 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; + } + + result = await showDialog({ + title: title, + body: new GitPullPushDialog(model, operation, credentials.value), + buttons: [Dialog.okButton({ label: 'DISMISS' })] + }); + retry = true; + } + } + + /** Add git push command */ + commands.addCommand(CommandIDs.gitPush, { + label: 'Push to Remote', + caption: 'Push code to remote repository', + isVisible: () => model.pathRepository !== null, + execute: async () => { + await showGitOperationDialog(model, Operation.Push).catch(reason => { + console.error( + `Encountered an error when pushing changes. Error: ${reason}` + ); + }); + } + }); + + /** Add git pull command */ + commands.addCommand(CommandIDs.gitPull, { + label: 'Pull from Remote', + caption: 'Pull latest code from remote repository', + isVisible: () => model.pathRepository !== null, + execute: async () => { + await showGitOperationDialog(model, Operation.Pull).catch(reason => { + console.error( + `Encountered an error when pulling changes. Error: ${reason}` + ); + }); + } + }); } diff --git a/src/index.ts b/src/index.ts index 0054b0a5d..bf801893e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -145,6 +145,8 @@ function createGitMenu( [ CommandIDs.gitInit, CommandIDs.gitClone, + CommandIDs.gitPush, + CommandIDs.gitPull, CommandIDs.gitAddRemote, CommandIDs.gitTerminalCommand ].forEach(command => { diff --git a/src/tokens.ts b/src/tokens.ts index 9ccc28aed..a99234311 100644 --- a/src/tokens.ts +++ b/src/tokens.ts @@ -3,6 +3,7 @@ import { DocumentRegistry } from '@jupyterlab/docregistry'; import { Token, JSONObject } from '@lumino/coreutils'; import { IDisposable } from '@lumino/disposable'; import { ISignal } from '@lumino/signaling'; +import { CommandRegistry } from '@lumino/commands'; export const EXTENSION_ID = 'jupyter.extensions.git_plugin'; @@ -57,6 +58,8 @@ export interface IGitExtension extends IDisposable { */ readonly statusChanged: ISignal; + readonly commands: CommandRegistry | null; + /** * Make request to add one or all files into * the staging area in repository From 610ba2aaff5b4aa228c879913c513fd533961ffa Mon Sep 17 00:00:00 2001 From: Marc Udoff Date: Thu, 23 Jul 2020 12:07:00 -0400 Subject: [PATCH 3/5] isVisible->isEnabled --- src/gitMenuCommands.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/gitMenuCommands.ts b/src/gitMenuCommands.ts index ef1e6c6d9..cf9e16b10 100644 --- a/src/gitMenuCommands.ts +++ b/src/gitMenuCommands.ts @@ -68,7 +68,7 @@ export function addCommands( main.dispose(); } }, - isVisible: () => model.pathRepository !== null + isEnabled: () => model.pathRepository !== null }); /** Add open/go to git interface command */ @@ -101,7 +101,7 @@ export function addCommands( model.pathRepository = currentPath; } }, - isVisible: () => model.pathRepository === null + isEnabled: () => model.pathRepository === null }); /** Open URL externally */ @@ -135,7 +135,7 @@ export function addCommands( commands.addCommand(CommandIDs.gitAddRemote, { label: 'Add remote repository', caption: 'Add a Git remote repository', - isVisible: () => model.pathRepository !== null, + isEnabled: () => model.pathRepository !== null, execute: async args => { if (model.pathRepository === null) { console.warn('Not in a Git repository. Unable to add a remote.'); @@ -174,8 +174,7 @@ export function addCommands( execute: async () => { await doGitClone(model, fileBrowser.model.path); fileBrowser.model.refresh(); - }, - isVisible: () => model.pathRepository === null + } }); /** @@ -224,7 +223,7 @@ export function addCommands( commands.addCommand(CommandIDs.gitPush, { label: 'Push to Remote', caption: 'Push code to remote repository', - isVisible: () => model.pathRepository !== null, + isEnabled: () => model.pathRepository !== null, execute: async () => { await showGitOperationDialog(model, Operation.Push).catch(reason => { console.error( @@ -238,7 +237,7 @@ export function addCommands( commands.addCommand(CommandIDs.gitPull, { label: 'Pull from Remote', caption: 'Pull latest code from remote repository', - isVisible: () => model.pathRepository !== null, + isEnabled: () => model.pathRepository !== null, execute: async () => { await showGitOperationDialog(model, Operation.Pull).catch(reason => { console.error( From f437bcb63ce18251cba5135ea0e5a0d44610dc77 Mon Sep 17 00:00:00 2001 From: Marc Udoff Date: Thu, 23 Jul 2020 12:10:13 -0400 Subject: [PATCH 4/5] Consistent capatalization --- src/gitMenuCommands.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gitMenuCommands.ts b/src/gitMenuCommands.ts index cf9e16b10..1ac481673 100644 --- a/src/gitMenuCommands.ts +++ b/src/gitMenuCommands.ts @@ -133,7 +133,7 @@ export function addCommands( /** Command to add a remote Git repository */ commands.addCommand(CommandIDs.gitAddRemote, { - label: 'Add remote repository', + label: 'Add Remote Repository', caption: 'Add a Git remote repository', isEnabled: () => model.pathRepository !== null, execute: async args => { From c391b4737b23ac028b93ee5ceead6dd3ca2b8726 Mon Sep 17 00:00:00 2001 From: Frederic Collonval Date: Fri, 24 Jul 2020 10:32:50 +0200 Subject: [PATCH 5/5] Fix jest tests --- src/gitMenuCommands.ts | 66 +++++++++++++++----------- tests/test-components/Toolbar.spec.tsx | 51 +++++++++++++------- 2 files changed, 72 insertions(+), 45 deletions(-) diff --git a/src/gitMenuCommands.ts b/src/gitMenuCommands.ts index 1ac481673..ed203a479 100644 --- a/src/gitMenuCommands.ts +++ b/src/gitMenuCommands.ts @@ -177,6 +177,41 @@ export function addCommands( } }); + /** Add git push command */ + commands.addCommand(CommandIDs.gitPush, { + label: 'Push to Remote', + 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}` + ); + } + ); + } + }); + + /** Add git pull command */ + commands.addCommand(CommandIDs.gitPull, { + label: 'Pull from Remote', + 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}` + ); + } + ); + } + }); +} + +/* eslint-disable no-inner-declarations */ +namespace Private { /** * Displays an error dialog when a Git operation fails. * @@ -185,7 +220,7 @@ export function addCommands( * @param operation - Git operation name * @returns Promise for displaying a dialog */ - async function showGitOperationDialog( + export async function showGitOperationDialog( model: IGitExtension, operation: Operation ): Promise { @@ -218,32 +253,5 @@ export function addCommands( retry = true; } } - - /** Add git push command */ - commands.addCommand(CommandIDs.gitPush, { - label: 'Push to Remote', - caption: 'Push code to remote repository', - isEnabled: () => model.pathRepository !== null, - execute: async () => { - await showGitOperationDialog(model, Operation.Push).catch(reason => { - console.error( - `Encountered an error when pushing changes. Error: ${reason}` - ); - }); - } - }); - - /** Add git pull command */ - commands.addCommand(CommandIDs.gitPull, { - label: 'Pull from Remote', - caption: 'Pull latest code from remote repository', - isEnabled: () => model.pathRepository !== null, - execute: async () => { - await showGitOperationDialog(model, Operation.Pull).catch(reason => { - console.error( - `Encountered an error when pulling changes. Error: ${reason}` - ); - }); - } - }); } +/* eslint-enable no-inner-declarations */ diff --git a/tests/test-components/Toolbar.spec.tsx b/tests/test-components/Toolbar.spec.tsx index 15abf2526..692831531 100644 --- a/tests/test-components/Toolbar.spec.tsx +++ b/tests/test-components/Toolbar.spec.tsx @@ -1,10 +1,12 @@ import { refreshIcon } from '@jupyterlab/ui-components'; +import * as commands from '@lumino/commands'; import { shallow } from 'enzyme'; import 'jest'; import * as React from 'react'; import { ActionButton } from '../../src/components/ActionButton'; import { Toolbar } from '../../src/components/Toolbar'; import * as git from '../../src/git'; +import { CommandIDs } from '../../src/gitMenuCommands'; import { GitExtension } from '../../src/model'; import { pullIcon, pushIcon } from '../../src/style/icons'; import { @@ -12,10 +14,15 @@ import { toolbarMenuButtonClass } from '../../src/style/Toolbar'; +jest.mock('@lumino/commands'); jest.mock('../../src/git'); -async function createModel() { - const model = new GitExtension(); +async function createModel(commands?: commands.CommandRegistry) { + const app = { + commands, + shell: null as any + }; + const model = new GitExtension(app as any); jest.spyOn(model, 'currentBranch', 'get').mockReturnValue({ is_current_branch: true, @@ -303,17 +310,25 @@ describe('Toolbar', () => { describe('pull changes', () => { let model: GitExtension; + let mockedExecute: any; beforeEach(async () => { + const mockedCommands = commands as jest.Mocked; + mockedExecute = jest.fn(); + mockedCommands.CommandRegistry.mockImplementation(() => { + return { + execute: mockedExecute + } as any; + }); + const registry = new commands.CommandRegistry(); + const mock = git as jest.Mocked; mock.httpGitRequest.mockImplementation(request); - model = await createModel(); + model = await createModel(registry); }); it('should pull changes when the button to pull the latest changes is clicked', () => { - const spy = jest.spyOn(GitExtension.prototype, 'pull'); - const props = { model: model, branching: false, @@ -323,26 +338,32 @@ describe('Toolbar', () => { const button = node.find(`.${toolbarButtonClass}`).first(); button.simulate('click'); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith(undefined); - - spy.mockRestore(); + expect(mockedExecute).toHaveBeenCalledTimes(1); + expect(mockedExecute).toHaveBeenCalledWith(CommandIDs.gitPull); }); }); describe('push changes', () => { let model: GitExtension; + let mockedExecute: any; beforeEach(async () => { + const mockedCommands = commands as jest.Mocked; + mockedExecute = jest.fn(); + mockedCommands.CommandRegistry.mockImplementation(() => { + return { + execute: mockedExecute + } as any; + }); + const registry = new commands.CommandRegistry(); + const mock = git as jest.Mocked; mock.httpGitRequest.mockImplementation(request); - model = await createModel(); + model = await createModel(registry); }); it('should push changes when the button to push the latest changes is clicked', () => { - const spy = jest.spyOn(GitExtension.prototype, 'push'); - const props = { model: model, branching: false, @@ -352,10 +373,8 @@ describe('Toolbar', () => { const button = node.find(`.${toolbarButtonClass}`).at(1); button.simulate('click'); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith(undefined); - - spy.mockRestore(); + expect(mockedExecute).toHaveBeenCalledTimes(1); + expect(mockedExecute).toHaveBeenCalledWith(CommandIDs.gitPush); }); });