Skip to content

Commit

Permalink
feat: improve dirty diff (#1978)
Browse files Browse the repository at this point in the history
* feat: improve dirty diff styles

* feat: add close dirty diff widget shortcut

* fix: content position

* chore: header background

* feat: dirty diff i18n

* fix: test

* fix: test

* fix: git revert change error

* fix: test

* fix: test

* fix: test

* fix: test
  • Loading branch information
Aaaaash authored Nov 22, 2022
1 parent 40258f7 commit 1df4974
Show file tree
Hide file tree
Showing 18 changed files with 360 additions and 132 deletions.
5 changes: 3 additions & 2 deletions packages/core-browser/src/menu/next/menu.contrib.interface.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type vscode from 'vscode';

import { URI, ILineChange } from '@opensumi/ide-core-common';
import { URI } from '@opensumi/ide-core-common';
import type { IChange } from '@opensumi/monaco-editor-core/esm/vs/editor/common/diff/smartLinesDiffComputer';

// explorer/context
// 资源管理器 ctxmenu
Expand Down Expand Up @@ -58,7 +59,7 @@ export type ScmResourceStateContextCallback = (...args: [vscode.SourceControlRes
// diff editor 的最右侧 ellipsis 图标点击 dropdown
// 此处 Uri 是 git scheme 的
// todo: scm/change/title 是 diff widget 的菜单,这里不对待更新
export type ScmChangeTitleCallback = (...args: [URI, ILineChange[], number]) => void;
export type ScmChangeTitleCallback = (...args: [URI, IChange[], number]) => void;

// view/title
// 自定义 contributed view 的 inline actions/more actions
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/src/common/en-US.lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ export const localizationBundle = {
'scm.diff.change.next': 'Next Change',
'scm.diff.change.previous': 'Previous Change',
'scm.diff.toggle.renderSideBySide': 'Toggle Inline View',
'scm.dirtyDiff.changes': '{0} of {1} changes',

'debug.action.add.configuration': 'Add Configuration...',
'debug.action.no.configuration': 'No Configurations',
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/src/common/zh-CN.lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ export const localizationBundle = {
'scm.diff.change.next': '下一个变化',
'scm.diff.change.previous': '上一个变化',
'scm.diff.toggle.renderSideBySide': '切换内联视图',
'scm.dirtyDiff.changes': '第 {0} 个更改 (共 {1} 个)',

'debug.action.start': '启动调试',
'debug.action.no.configuration': '暂无配置',
Expand Down
4 changes: 3 additions & 1 deletion packages/monaco/__mocks__/monaco/editor/code-editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ export class MockedCodeEditor extends Disposable implements monaco.editor.ICodeE
throw new Error('Method not implemented.');
}
getOption(_: any): any {
throw new Error('Method not implemented.');
const options = new Map();
options.set(60, 30);
return options;
}
getContentWidth(): number {
throw new Error('Method not implemented.');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Injectable } from '@opensumi/di';
import { IContextKeyService } from '@opensumi/ide-core-browser/lib/context-key';
import { ILineChange, URI } from '@opensumi/ide-core-common';
import { OverviewRulerLane, IDocPersistentCacheProvider } from '@opensumi/ide-editor';
import { EmptyDocCacheImpl } from '@opensumi/ide-editor/src/browser';
Expand All @@ -8,6 +9,7 @@ import { ITextModel } from '@opensumi/ide-monaco/lib/browser/monaco-api/types';

import { createBrowserInjector } from '../../../../../tools/dev-tool/src/injector-helper';
import { MockInjector } from '../../../../../tools/dev-tool/src/mock-injector';
import { MockContextKeyService } from '../../../../monaco/__mocks__/monaco.context-key.service';
import { DirtyDiffDecorator } from '../../../src/browser/dirty-diff/dirty-diff-decorator';
import { DirtyDiffModel } from '../../../src/browser/dirty-diff/dirty-diff-model';
import { SCMPreferences } from '../../../src/browser/scm-preference';
Expand All @@ -31,6 +33,10 @@ describe('test for scm/src/browser/dirty-diff/dirty-diff-decorator.ts', () => {
injector = createBrowserInjector(
[],
new MockInjector([
{
token: IContextKeyService,
useClass: MockContextKeyService,
},
{
token: IDocPersistentCacheProvider,
useClass: EmptyDocCacheImpl,
Expand All @@ -55,7 +61,7 @@ describe('test for scm/src/browser/dirty-diff/dirty-diff-decorator.ts', () => {
const spy = jest.spyOn(monacoModel, 'deltaDecorations');

// ChangeType#Add
const change0: ILineChange = [10, 0, 111, 0, []];
const change0: ILineChange = [10, 10, 111, 0, []];
dirtyDiffModel['_changes'] = [change0];
dirtyDiffModel['_onDidChange'].fire([
{
Expand All @@ -68,10 +74,13 @@ describe('test for scm/src/browser/dirty-diff/dirty-diff-decorator.ts', () => {
expect(spy).toHaveBeenCalledTimes(1);
const decos = spy.mock.calls[0][1];
expect(decos.length).toBe(1);
const startLineNumber = change0[2];
const endLineNumber = change0[3] - 1 || startLineNumber - 1;

expect(decos[0].range).toEqual({
startLineNumber: change0[2],
startLineNumber,
startColumn: 1,
endLineNumber: change0[2],
endLineNumber,
endColumn: 1,
});
expect(decos[0].options.linesDecorationsClassName).toBe('dirty-diff-glyph dirty-diff-added');
Expand Down Expand Up @@ -99,7 +108,7 @@ describe('test for scm/src/browser/dirty-diff/dirty-diff-decorator.ts', () => {
const spy = jest.spyOn(monacoModel, 'deltaDecorations');

// ChangeType#Delete
const change0: ILineChange = [1, 10, 111, 0, []];
const change0: ILineChange = [10, 11, 111, 111, []];
dirtyDiffModel['_changes'] = [change0];
dirtyDiffModel['_onDidChange'].fire([
{
Expand All @@ -112,10 +121,11 @@ describe('test for scm/src/browser/dirty-diff/dirty-diff-decorator.ts', () => {
expect(spy).toHaveBeenCalledTimes(1);
const decos = spy.mock.calls[0][1];
expect(decos.length).toBe(1);
const startLineNumber = change0[2];
expect(decos[0].range).toEqual({
startLineNumber: change0[2],
startLineNumber: startLineNumber - 1,
startColumn: Number.MAX_VALUE,
endLineNumber: change0[2],
endLineNumber: startLineNumber > 0 ? startLineNumber - 1 : startLineNumber,
endColumn: Number.MAX_VALUE,
});
expect(decos[0].options.linesDecorationsClassName).toBeNull();
Expand Down Expand Up @@ -156,10 +166,13 @@ describe('test for scm/src/browser/dirty-diff/dirty-diff-decorator.ts', () => {
expect(spy).toHaveBeenCalledTimes(1);
const decos = spy.mock.calls[0][1];
expect(decos.length).toBe(1);
const startLineNumber = change0[2];
const endLineNumber = change0[3] - 1 || startLineNumber - 1;

expect(decos[0].range).toEqual({
startLineNumber: change0[2],
startLineNumber,
startColumn: 1,
endLineNumber: change0[3],
endLineNumber,
endColumn: 1,
});
expect(decos[0].options.linesDecorationsClassName).toBeNull();
Expand Down
18 changes: 12 additions & 6 deletions packages/scm/__tests__/browser/dirty-diff/dirty-diff-model.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Injectable, Autowired, INJECTOR_TOKEN, Injector } from '@opensumi/di';
import { IContextKeyService } from '@opensumi/ide-core-browser/lib/context-key';
import { toDisposable, Event, CommandService, positionToRange, URI, ILineChange } from '@opensumi/ide-core-common';
import { IDocPersistentCacheProvider } from '@opensumi/ide-editor';
import { EditorCollectionService } from '@opensumi/ide-editor';
Expand All @@ -15,6 +16,7 @@ import { StandaloneServices } from '@opensumi/monaco-editor-core/esm/vs/editor/s
import { createBrowserInjector } from '../../../../../tools/dev-tool/src/injector-helper';
import { MockInjector } from '../../../../../tools/dev-tool/src/mock-injector';
import { createMockedMonaco } from '../../../../monaco/__mocks__/monaco';
import { MockContextKeyService } from '../../../../monaco/__mocks__/monaco.context-key.service';
import { SCMService, ISCMRepository } from '../../../src';
import { DirtyDiffModel } from '../../../src/browser/dirty-diff/dirty-diff-model';
import { DirtyDiffWidget } from '../../../src/browser/dirty-diff/dirty-diff-widget';
Expand Down Expand Up @@ -95,6 +97,10 @@ describe('scm/src/browser/dirty-diff/dirty-diff-model.ts', () => {
injector = createBrowserInjector(
[],
new MockInjector([
{
token: IContextKeyService,
useClass: MockContextKeyService,
},
{
token: IDocPersistentCacheProvider,
useClass: EmptyDocCacheImpl,
Expand Down Expand Up @@ -435,11 +441,11 @@ describe('scm/src/browser/dirty-diff/dirty-diff-model.ts', () => {

// monacoEditor.revealLineInCenter
expect(revealLineInCenterSpy).toBeCalledTimes(1);
expect(revealLineInCenterSpy).toBeCalledWith(12 - 9);
expect(revealLineInCenterSpy).toBeCalledWith(7);

expect(dirtyDiffWidget.currentIndex).toBe(1);
expect(dirtyDiffWidget.currentRange).toEqual(positionToRange(12));
expect(dirtyDiffWidget.currentHeightInLines).toBe(18);
expect(dirtyDiffWidget.currentRange).toEqual(positionToRange(11));
expect(dirtyDiffWidget.currentHeightInLines).toBe(10);

// this.onDidChange
dirtyDiffModel['_changes'].unshift(change0);
Expand All @@ -452,13 +458,13 @@ describe('scm/src/browser/dirty-diff/dirty-diff-model.ts', () => {
]);

expect(dirtyDiffWidget.currentIndex).toBe(2);
expect(dirtyDiffWidget.currentRange).toEqual(positionToRange(12));
expect(dirtyDiffWidget.currentHeightInLines).toBe(18);
expect(dirtyDiffWidget.currentRange).toEqual(positionToRange(11));
expect(dirtyDiffWidget.currentHeightInLines).toBe(10);

// originalEditor.monacoEditor.onDidChangeModelContent
originalMonacoEditor['_onDidChangeModelContent'].fire();
expect(relayoutSpy).toBeCalledTimes(1);
expect(relayoutSpy).toBeCalledWith(18);
expect(relayoutSpy).toBeCalledWith(10);

// widget.onDispose
dirtyDiffWidget.dispose();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,35 @@
import { Injectable, Autowired, INJECTOR_TOKEN, Injector } from '@opensumi/di';
import { positionToRange, URI, CommandService, ILineChange } from '@opensumi/ide-core-common';
import { IContextKeyService } from '@opensumi/ide-core-browser/lib/context-key';
import {
positionToRange,
URI,
CommandService,
ILineChange,
registerLocalizationBundle,
} from '@opensumi/ide-core-common';
import { IDocPersistentCacheProvider } from '@opensumi/ide-editor';
import { EmptyDocCacheImpl, IEditorDocumentModelService } from '@opensumi/ide-editor/src/browser';
import { IEditorDocumentModel } from '@opensumi/ide-editor/src/browser/';
import { EditorDocumentModel } from '@opensumi/ide-editor/src/browser/doc-model/main';
import { toChange } from '@opensumi/ide-scm/lib/browser/dirty-diff/dirty-diff-util';

import { createBrowserInjector } from '../../../../../tools/dev-tool/src/injector-helper';
import { MockInjector } from '../../../../../tools/dev-tool/src/mock-injector';
import { createMockedMonaco } from '../../../../monaco/__mocks__/monaco';
import { MockContextKeyService } from '../../../../monaco/__mocks__/monaco.context-key.service';
import { SCMService } from '../../../src';
import { DirtyDiffModel } from '../../../src/browser/dirty-diff/dirty-diff-model';
import { DirtyDiffWidget } from '../../../src/browser/dirty-diff/dirty-diff-widget';

registerLocalizationBundle({
languageId: 'zh-CN',
contents: {
'scm.dirtyDiff.changes': '第 {0} 个更改 (共 {1} 个)',
},
languageName: 'Chinese',
localizedLanguageName: '中文(中国)',
});

@Injectable()
class MockEditorDocumentModelService {
@Autowired(INJECTOR_TOKEN)
Expand Down Expand Up @@ -53,6 +71,10 @@ describe('scm/src/browser/dirty-diff/dirty-diff-widget.ts', () => {
injector = createBrowserInjector(
[],
new MockInjector([
{
token: IContextKeyService,
useClass: MockContextKeyService,
},
{
token: IDocPersistentCacheProvider,
useClass: EmptyDocCacheImpl,
Expand Down Expand Up @@ -166,20 +188,30 @@ describe('scm/src/browser/dirty-diff/dirty-diff-widget.ts', () => {
const actionList = Array.from(actions.children) as HTMLElement[];
expect(actionList.length).toBe(5);
expect(actionList.map((n) => n.className)).toEqual(
['plus', 'rollback', 'up', 'down', 'close'].map((n) => `kaitian-icon kticon-${n}`),
['add', 'discard', 'arrow-up', 'arrow-down', 'close'].map((n) => `kt-icon codicon codicon-${n}`),
);
// onclick test

// add
actionList[0].click();
expect(fakeExecCmd).toBeCalledTimes(1);
expect(fakeExecCmd.mock.calls[0]).toEqual(['git.stageChange', URI.file('/test/workspace/abc.ts'), changes, 1]);
expect(fakeExecCmd.mock.calls[0]).toEqual([
'git.stageChange',
URI.file('/test/workspace/abc.ts'),
changes.map(toChange),
1,
]);
expect(fakeDispose).toHaveBeenCalledTimes(1);

// revert
actionList[1].click();
expect(fakeExecCmd).toBeCalledTimes(2);
expect(fakeExecCmd.mock.calls[1]).toEqual(['git.revertChange', URI.file('/test/workspace/abc.ts'), changes, 1]);
expect(fakeExecCmd.mock.calls[1]).toEqual([
'git.revertChange',
URI.file('/test/workspace/abc.ts'),
changes.map(toChange),
1,
]);
expect(fakeDispose).toHaveBeenCalledTimes(2);

// next
Expand Down Expand Up @@ -225,13 +257,13 @@ describe('scm/src/browser/dirty-diff/dirty-diff-widget.ts', () => {
const detail = title.children[0];
expect(detail.tagName).toBe('SPAN');
expect(detail.className).toBe('dirty-diff-widget-title-detail');
expect((detail as HTMLElement).innerText).toBe('第 2 个更改共 4 个');
expect((detail as HTMLElement).innerText).toBe('第 2 个更改 (共 4 个)');

dirtyDiffWidget.updateCurrent(4);

dirtyDiffWidget['applyStyle']();
// 上一个 children[0] 已经被移除了
expect((title.children[0] as HTMLElement).innerText).toBe('第 4 个更改共 4 个');
expect((title.children[0] as HTMLElement).innerText).toBe('第 4 个更改 (共 4 个)');
});

it('ok for relayout', () => {
Expand Down
23 changes: 8 additions & 15 deletions packages/scm/__tests__/browser/dirty-diff/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Autowired, Injectable, Injector, INJECTOR_TOKEN } from '@opensumi/di';
import { PreferenceChange } from '@opensumi/ide-core-browser';
import { IContextKeyService, PreferenceChange } from '@opensumi/ide-core-browser';
import {
DisposableCollection,
PreferenceScope,
Expand All @@ -26,6 +26,7 @@ import * as monaco from '@opensumi/monaco-editor-core/esm/vs/editor/editor.api';

import { createBrowserInjector } from '../../../../../tools/dev-tool/src/injector-helper';
import { MockInjector } from '../../../../../tools/dev-tool/src/mock-injector';
import { MockContextKeyService } from '../../../../monaco/__mocks__/monaco.context-key.service';
import { IDirtyDiffWorkbenchController } from '../../../src';
import { DirtyDiffWorkbenchController, DirtyDiffItem } from '../../../src/browser/dirty-diff';
import { DirtyDiffDecorator } from '../../../src/browser/dirty-diff/dirty-diff-decorator';
Expand Down Expand Up @@ -92,6 +93,10 @@ describe('scm/src/browser/dirty-diff/index.ts', () => {
injector = createBrowserInjector(
[],
new Injector([
{
token: IContextKeyService,
useClass: MockContextKeyService,
},
{
token: IDocPersistentCacheProvider,
useClass: EmptyDocCacheImpl,
Expand Down Expand Up @@ -423,20 +428,8 @@ describe('scm/src/browser/dirty-diff/index.ts', () => {
const dirtyDiffDecorator = injector.get(DirtyDiffDecorator, [editorModel, dirtyDiffModel]);
const dirtyDiffWidget = injector.get(DirtyDiffWidget, [monacoEditor, dirtyDiffModel, commandService]);

const change0: ILineChange = [
11,
11,
11,
11,
[],
];
const change1: ILineChange = [
12,
12,
12,
12,
[],
];
const change0: ILineChange = [11, 11, 11, 11, []];
const change1: ILineChange = [12, 12, 12, 12, []];

dirtyDiffModel['_changes'] = [change0, change1];
dirtyDiffWidget.updateCurrent(1);
Expand Down
Loading

0 comments on commit 1df4974

Please sign in to comment.