Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

scm/git UI fixes #5254

Merged
merged 1 commit into from
May 29, 2019
Merged

scm/git UI fixes #5254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion examples/browser/test/left-panel/left-panel.ui-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ before(() => {
});

describe('theia left panel', () => {
it("should show 'Explorer'", () => {
it("should show 'Explorer' and 'SCM'", () => {
expect(leftPanel.doesTabExist('Explorer')).to.be.true;
expect(leftPanel.doesTabExist('Source Control')).to.be.true;
});

describe('files tab', () => {
Expand All @@ -48,4 +49,16 @@ describe('theia left panel', () => {
expect(leftPanel.isTabActive('Explorer')).to.be.false;
});
});

describe('SCM tab', () => {
it('should open/close the SCM tab', () => {
leftPanel.openCloseTab('Source Control');
expect(leftPanel.isScmContainerVisible()).to.be.true;
expect(leftPanel.isTabActive('Source Control')).to.be.true;

leftPanel.openCloseTab('Source Control');
expect(leftPanel.isScmContainerVisible()).to.be.false;
expect(leftPanel.isTabActive('Source Control')).to.be.false;
});
});
});
2 changes: 1 addition & 1 deletion examples/browser/test/top-panel/top-panel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export class TopPanel {

toggleScmView() {
this.clickMenuTab('View');
this.clickSubMenu('Scm');
this.clickSubMenu('SCM');
}

toggleGitHistoryView() {
Expand Down
3 changes: 0 additions & 3 deletions packages/core/src/common/command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ export interface Command {
* A category of this command.
*/
category?: string;

// tslint:disable-next-line:no-any
props?: { [key: string]: any }
}

export namespace Command {
Expand Down
108 changes: 77 additions & 31 deletions packages/git/src/browser/git-contribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,7 @@ export namespace GIT_COMMANDS {
id: 'git-commit-add-sign-off',
label: 'Add Signed-off-by',
iconClass: 'fa fa-pencil-square-o',
category: 'navigation',
props: { ['group']: 'navigation' }
category: 'Git'
};
export const COMMIT_AMEND = {
id: 'git.commit.amend'
Expand All @@ -137,7 +136,8 @@ export namespace GIT_COMMANDS {
export const OPEN_FILE: Command = {
id: 'git.open.file',
category: 'Git',
label: 'Open File'
label: 'Open File',
iconClass: 'theia-open-file-icon'
};
export const OPEN_CHANGED_FILE: Command = {
id: 'git.open.changed.file',
Expand All @@ -148,7 +148,8 @@ export namespace GIT_COMMANDS {
export const OPEN_CHANGES: Command = {
id: 'git.open.changes',
category: 'Git',
label: 'Open Changes'
label: 'Open Changes',
iconClass: 'theia-open-change-icon'
akosyakov marked this conversation as resolved.
Show resolved Hide resolved
};
export const SYNC = {
id: 'git.sync',
Expand All @@ -160,39 +161,39 @@ export namespace GIT_COMMANDS {
};
export const STAGE = {
id: 'git.stage',
category: 'Git',
label: 'Stage Changes',
iconClass: 'fa fa-plus'
};
export const STAGE_ALL = {
id: 'git.stage.all',
category: 'Git',
label: 'Stage All Changes',
iconClass: 'fa fa-plus',
category: 'inline',
props: { ['group']: 'inline' }
};
export const UNSTAGE = {
id: 'git.unstage',
iconClass: 'fa fa-minus',
category: 'Git',
label: 'Unstage Changes'
};
export const UNSTAGE_ALL = {
id: 'git.unstage.all',
iconClass: 'fa fa-minus',
category: 'Git',
label: 'Unstage All',
category: 'inline',
props: { ['group']: 'inline' }
};
export const DISCARD = {
id: 'git.discard',
iconClass: 'fa fa-undo',
category: 'Git',
label: 'Discard Changes'
};
export const DISCARD_ALL = {
id: 'git.discard.all',
iconClass: 'fa fa-undo',
category: 'Git',
label: 'Discard All Changes',
category: 'inline',
props: { ['group']: 'inline' }
};
export const STASH = {
id: 'git.stash',
Expand Down Expand Up @@ -228,8 +229,7 @@ export namespace GIT_COMMANDS {
id: 'git-refresh',
label: 'Refresh',
iconClass: 'fa fa-refresh',
category: 'navigation',
props: { ['group']: 'navigation' }
category: 'Git'
};
}

Expand All @@ -242,12 +242,13 @@ export class GitContribution implements
ScmResourceCommandContribution,
ScmGroupCommandContribution {

static GIT_SELECTED_REPOSITORY = 'git-selected-repository';
static GIT_REPOSITORY_STATUS = 'git-repository-status';
static GIT_CHECKOUT = 'git.checkout';
static GIT_SYNC_STATUS = 'git-sync-status';

protected toDispose = new DisposableCollection();

protected readonly icons: Map<string, ScmCommand> = new Map();

private dirtyRepositories: Repository[] = [];
private scmProviders: ScmProvider[] = [];

Expand Down Expand Up @@ -282,7 +283,7 @@ export class GitContribution implements
if (repository) {
this.repositoryProvider.selectedRepository = repository;
} else {
this.statusBar.removeElement(GitContribution.GIT_REPOSITORY_STATUS);
this.statusBar.removeElement(GitContribution.GIT_CHECKOUT);
this.statusBar.removeElement(GitContribution.GIT_SYNC_STATUS);
}
});
Expand Down Expand Up @@ -310,14 +311,14 @@ export class GitContribution implements
this.getGroups(status, provider).then(groups => {
provider.groups = groups;
provider.fireChangeStatusBarCommands([{
id: GIT_COMMANDS.CHECKOUT.id,
id: GitContribution.GIT_CHECKOUT,
text: `$(code-fork) ${branch}${dirty}`,
command: GIT_COMMANDS.CHECKOUT.id
}]);
provider.fireChangeResources();
this.updateSyncStatusBarEntry(event.source.localUri);
});
}
this.updateSyncStatusBarEntry(event.source.localUri);
});
this.syncService.onDidChange(() => this.updateSyncStatusBarEntry(
this.repositoryProvider.selectedRepository
Expand Down Expand Up @@ -414,7 +415,10 @@ export class GitContribution implements
this.dirtyRepositories
.find(dirtyRepo => this.repositoryProvider.allRepositories.every(repo => repo.localUri !== dirtyRepo.localUri));
if (removed) {
this.dirtyRepositories.push(removed);
const i = this.dirtyRepositories.indexOf(removed, 0);
if (i > -1) {
this.dirtyRepositories.splice(i, 1);
}
const removedScmRepo = this.scmService.repositories.find(scmRepo => scmRepo.provider.rootUri === removed.localUri);
if (removedScmRepo) {
removedScmRepo.dispose();
Expand All @@ -437,7 +441,7 @@ export class GitContribution implements
const provider = new ScmProviderImpl('Git', uri.substring(uri.lastIndexOf('/') + 1), uri, amendSupport);
this.scmProviders.push(provider);
const repo = this.scmService.registerScmProvider(provider, disposableCollection);
repo.input.placeholder = 'Commit Message';
repo.input.placeholder = 'Commit message';
repo.input.validateInput = async input => {
const validate = await this.commitMessageValidator.validate(input);
if (validate) {
Expand Down Expand Up @@ -734,13 +738,11 @@ export class GitContribution implements
registry.registerItem({
id: GIT_COMMANDS.OPEN_FILE.id,
command: GIT_COMMANDS.OPEN_FILE.id,
text: '$(file-o)',
tooltip: GIT_COMMANDS.OPEN_FILE.label
});
registry.registerItem({
id: GIT_COMMANDS.OPEN_CHANGES.id,
command: GIT_COMMANDS.OPEN_CHANGES.id,
text: '$(files-o)',
tooltip: GIT_COMMANDS.OPEN_CHANGES.label
});
}
Expand Down Expand Up @@ -800,7 +802,7 @@ export class GitContribution implements
const scmProvider = this.scmProviders.find(provider => provider.rootUri === repositoryUri);
if (scmProvider) {
(scmProvider as ScmProviderImpl).fireChangeStatusBarCommands([{
id: 'vcs-sync-status',
id: GitContribution.GIT_SYNC_STATUS,
text: entry.text,
tooltip: entry.tooltip,
command: entry.command,
Expand All @@ -824,7 +826,7 @@ export class GitContribution implements
const { upstreamBranch, aheadBehind } = status;
if (upstreamBranch) {
return {
text: '$(refresh)' + (aheadBehind ? ` ${aheadBehind.behind} $(arrow-down) ${aheadBehind.ahead} $(arrow-up)` : ''),
text: '$(refresh)' + (aheadBehind && (aheadBehind.ahead + aheadBehind.behind) > 0 ? ` ${aheadBehind.behind}↓ ${aheadBehind.ahead}↑` : ''),
command: GIT_COMMANDS.SYNC.id,
tooltip: 'Synchronize Changes'
};
Expand All @@ -837,20 +839,64 @@ export class GitContribution implements
}

registerScmTitleCommands(registry: ScmTitleCommandRegistry): void {
registry.registerCommand({ command: GIT_COMMANDS.REFRESH.id });
registry.registerCommand({ command: GIT_COMMANDS.COMMIT_ADD_SIGN_OFF.id });
registry.registerItem({ command: GIT_COMMANDS.REFRESH.id, group: 'navigation' });
registry.registerItem({ command: GIT_COMMANDS.COMMIT_ADD_SIGN_OFF.id, group: 'navigation'});
}

registerScmResourceCommands(registry: ScmResourceCommandRegistry): void {
registry.registerCommands('Changes', [GIT_COMMANDS.OPEN_CHANGED_FILE.id, GIT_COMMANDS.DISCARD.id, GIT_COMMANDS.STAGE.id]);
registry.registerCommands('Staged changes', [GIT_COMMANDS.OPEN_CHANGED_FILE.id, GIT_COMMANDS.UNSTAGE.id]);
registry.registerCommands('Merged Changes', [GIT_COMMANDS.OPEN_CHANGED_FILE.id, GIT_COMMANDS.DISCARD.id, GIT_COMMANDS.STAGE.id]);
registry.registerItems('Changes', [
{
command: GIT_COMMANDS.OPEN_CHANGED_FILE.id,
group: 'navigation'
},
{
command: GIT_COMMANDS.DISCARD.id,
group: 'navigation'
},
{
command: GIT_COMMANDS.STAGE.id,
group: 'navigation'
}
]);
registry.registerItems('Staged changes', [
{
command: GIT_COMMANDS.OPEN_CHANGED_FILE.id,
group: 'navigation'
},
{
command: GIT_COMMANDS.UNSTAGE.id,
group: 'navigation'
}
]);
registry.registerItems('Merged Changes', [
{
command: GIT_COMMANDS.OPEN_CHANGED_FILE.id,
group: 'navigation'
},
{
command: GIT_COMMANDS.DISCARD.id,
group: 'navigation'
},
{
command: GIT_COMMANDS.STAGE.id,
group: 'navigation'
}
]);
}

registerScmGroupCommands(registry: ScmGroupCommandRegistry): void {
registry.registerCommands('Changes', [GIT_COMMANDS.DISCARD_ALL.id, GIT_COMMANDS.STAGE_ALL.id]);
registry.registerCommands('Staged changes', [GIT_COMMANDS.UNSTAGE_ALL.id]);
registry.registerCommands('Merged Changes', [GIT_COMMANDS.STAGE_ALL.id]);
registry.registerItems('Changes', [
{
command: GIT_COMMANDS.DISCARD_ALL.id,
group: 'inline'
},
{
command: GIT_COMMANDS.STAGE_ALL.id,
group: 'inline'
}
]);
registry.registerItems('Staged changes', [{ command: GIT_COMMANDS.UNSTAGE_ALL.id, group: 'inline' }]);
registry.registerItems('Merged Changes', [{ command: GIT_COMMANDS.STAGE_ALL.id, group: 'inline' }]);
}
}
export interface GitOpenFileOptions {
Expand Down
2 changes: 2 additions & 0 deletions packages/git/src/browser/git-repository-provider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { LocalStorageService, StorageService } from '@theia/core/lib/browser';
import { GitRepositoryProvider } from './git-repository-provider';
import * as sinon from 'sinon';
import * as chai from 'chai';
import { ScmService } from '@theia/scm/lib/browser';
const expect = chai.expect;

disableJSDOM();
Expand Down Expand Up @@ -90,6 +91,7 @@ describe('GitRepositoryProvider', () => {
testContainer.bind(FileSystem).toConstantValue(mockFilesystem);
testContainer.bind(FileSystemWatcher).toConstantValue(mockFileSystemWatcher);
testContainer.bind(StorageService).toConstantValue(mockStorageService);
testContainer.bind(ScmService).toSelf().inSingletonScope();

sinon.stub(mockWorkspaceService, 'onWorkspaceChanged').value(mockRootChangeEmitter.event);
sinon.stub(mockFileSystemWatcher, 'onFilesChanged').value(mockFileChangeEmitter.event);
Expand Down
6 changes: 5 additions & 1 deletion packages/git/src/browser/git-repository-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { DisposableCollection, Event, Emitter } from '@theia/core';
import { StorageService } from '@theia/core/lib/browser';
import URI from '@theia/core/lib/common/uri';
import { FileSystemWatcher } from '@theia/filesystem/lib/browser/filesystem-watcher';

import { ScmService } from '@theia/scm/lib/browser';
import debounce = require('lodash.debounce');

export interface GitRefreshOptions {
Expand All @@ -43,6 +43,7 @@ export class GitRepositoryProvider {
@inject(WorkspaceService) protected readonly workspaceService: WorkspaceService,
@inject(FileSystemWatcher) protected readonly watcher: FileSystemWatcher,
@inject(FileSystem) protected readonly fileSystem: FileSystem,
@inject(ScmService) protected readonly scmService: ScmService,
@inject(StorageService) protected readonly storageService: StorageService
) {
this.initialize();
Expand Down Expand Up @@ -93,6 +94,9 @@ export class GitRepositoryProvider {
* Sets or un-sets the repository.
*/
set selectedRepository(repository: Repository | undefined) {
if (!repository) {
Copy link
Member

@akosyakov akosyakov May 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only undefined should be accepted?

this.scmService.selectedRepository = undefined;
}
this._selectedRepository = repository;
Copy link
Member

@akosyakov akosyakov May 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • What is the difference between selectedRepository in GitRepositoryProvider and ScmService?
  • Should not they point to the same repository? Can GitRepositoryProvider delegate to ScmService for selectedRepository and onDidChangeRepository event?

this.storageService.setData<Repository | undefined>(this.selectedRepoStorageKey, repository);
this.fireDidChangeRepository();
Expand Down
9 changes: 3 additions & 6 deletions packages/plugin-ext/src/main/browser/menus/menus-contribution-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class MenusContributionPointHandler {
execute: () => this.commands.executeCommand(menuAction.command)
});

this.scmTitleCommandRegistry.registerCommand({ command: id, when: menuAction.when });
this.scmTitleCommandRegistry.registerItem({ command: id, when: menuAction.when, group: menuAction.group });

if (menuAction.group && menuAction.group !== 'navigation') {
const action: MenuAction = { commandId: id };
Expand All @@ -137,7 +137,6 @@ export class MenusContributionPointHandler {
command.iconClass = pluginCommand.iconClass;
command.category = pluginCommand.category;
command.label = pluginCommand.label;
command.props = { ['group']: menuAction.group };
});
}

Expand All @@ -157,15 +156,14 @@ export class MenusContributionPointHandler {
if (action.group !== 'inline') {
this.menuRegistry.registerMenuAction(['scm-group-context-menu_' + group], { commandId: id });
} else {
this.scmGroupCommandRegistry.registerCommand(group, id);
this.scmGroupCommandRegistry.registerItem(group, { command: id, group: action.group });
}
}

this.onDidRegisterCommand(action.command, pluginCommand => {
command.iconClass = pluginCommand.iconClass;
command.category = pluginCommand.category;
command.label = pluginCommand.label;
command.props = { ['group']: action.group };
});
}

Expand All @@ -183,7 +181,7 @@ export class MenusContributionPointHandler {
group = group.substring(0, group.indexOf(' &&'));
}
if (action.group && action.group.startsWith('inline')) {
this.scmResourceCommandRegistry.registerCommand(group, id);
this.scmResourceCommandRegistry.registerItem(group, {command: id, group: 'inline'});
} else {
this.menuRegistry.registerMenuAction(['scm-resource-context-menu_' + group], { commandId: id });
}
Expand All @@ -193,7 +191,6 @@ export class MenusContributionPointHandler {
command.iconClass = pluginCommand.iconClass;
command.category = pluginCommand.category;
command.label = pluginCommand.label;
command.props = { ['group']: action.group };
});
}

Expand Down
Loading