Skip to content

Commit

Permalink
Merge pull request #740 from fcollonval/auto-backport-of-pr-676-on-0.…
Browse files Browse the repository at this point in the history
…11.x

Backport PR #676: Update open files when Git commands modify them
fcollonval authored Aug 23, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
2 parents a78b0c8 + a528bb7 commit 67d1896
Showing 6 changed files with 176 additions and 17 deletions.
6 changes: 3 additions & 3 deletions jupyterlab_git/git.py
Original file line number Diff line number Diff line change
@@ -192,7 +192,7 @@ async def config(self, top_repo_path, **kwargs):

return response

async def changed_files(self, base=None, remote=None, single_commit=None):
async def changed_files(self, current_path, base=None, remote=None, single_commit=None):
"""Gets the list of changed files between two Git refs, or the files changed in a single commit
There are two reserved "refs" for the base
@@ -212,7 +212,7 @@ async def changed_files(self, base=None, remote=None, single_commit=None):
}
"""
if single_commit:
cmd = ["git", "diff", "{}^!".format(single_commit), "--name-only", "-z"]
cmd = ["git", "diff", single_commit, "--name-only", "-z"]
elif base and remote:
if base == "WORKING":
cmd = ["git", "diff", remote, "--name-only", "-z"]
@@ -227,7 +227,7 @@ async def changed_files(self, base=None, remote=None, single_commit=None):

response = {}
try:
code, output, error = await execute(cmd, cwd=self.root_dir)
code, output, error = await execute(cmd, cwd=os.path.join(self.root_dir, current_path))
except subprocess.CalledProcessError as e:
response["code"] = e.returncode
response["message"] = e.output.decode("utf-8")
22 changes: 11 additions & 11 deletions jupyterlab_git/tests/test_diff.py
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@
async def test_changed_files_invalid_input():
with pytest.raises(tornado.web.HTTPError):
await Git(FakeContentManager("/bin")).changed_files(
base="64950a634cd11d1a01ddfedaeffed67b531cb11e"
current_path="test-path", base="64950a634cd11d1a01ddfedaeffed67b531cb11e"
)


@@ -29,7 +29,7 @@ async def test_changed_files_single_commit():

# When
actual_response = await Git(FakeContentManager("/bin")).changed_files(
single_commit="64950a634cd11d1a01ddfedaeffed67b531cb11e"
current_path="test-path", single_commit="64950a634cd11d1a01ddfedaeffed67b531cb11e^!"
)

# Then
@@ -41,7 +41,7 @@ async def test_changed_files_single_commit():
"--name-only",
"-z",
],
cwd="/bin",
cwd="/bin/test-path",
)
assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response

@@ -56,12 +56,12 @@ async def test_changed_files_working_tree():

# When
actual_response = await Git(FakeContentManager("/bin")).changed_files(
base="WORKING", remote="HEAD"
current_path="test-path", base="WORKING", remote="HEAD"
)

# Then
mock_execute.assert_called_once_with(
["git", "diff", "HEAD", "--name-only", "-z"], cwd="/bin"
["git", "diff", "HEAD", "--name-only", "-z"], cwd="/bin/test-path"
)
assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response

@@ -76,12 +76,12 @@ async def test_changed_files_index():

# When
actual_response = await Git(FakeContentManager("/bin")).changed_files(
base="INDEX", remote="HEAD"
current_path="test-path", base="INDEX", remote="HEAD"
)

# Then
mock_execute.assert_called_once_with(
["git", "diff", "--staged", "HEAD", "--name-only", "-z"], cwd="/bin"
["git", "diff", "--staged", "HEAD", "--name-only", "-z"], cwd="/bin/test-path"
)
assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response

@@ -96,12 +96,12 @@ async def test_changed_files_two_commits():

# When
actual_response = await Git(FakeContentManager("/bin")).changed_files(
base="HEAD", remote="origin/HEAD"
current_path = "test-path", base="HEAD", remote="origin/HEAD"
)

# Then
mock_execute.assert_called_once_with(
["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin"
["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin/test-path"
)
assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response

@@ -114,12 +114,12 @@ async def test_changed_files_git_diff_error():

# When
actual_response = await Git(FakeContentManager("/bin")).changed_files(
base="HEAD", remote="origin/HEAD"
current_path="test-path", base="HEAD", remote="origin/HEAD"
)

# Then
mock_execute.assert_called_once_with(
["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin"
["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin/test-path"
)
assert {"code": 128, "message": "error message"} == actual_response

10 changes: 9 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import {
} from '@jupyterlab/application';
import { IChangedArgs, ISettingRegistry } from '@jupyterlab/coreutils';
import { Dialog, showErrorMessage } from '@jupyterlab/apputils';
import { IDocumentManager } from '@jupyterlab/docmanager';
import { FileBrowserModel, IFileBrowserFactory } from '@jupyterlab/filebrowser';
import { IMainMenu } from '@jupyterlab/mainmenu';
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
@@ -32,6 +33,7 @@ const plugin: JupyterFrontEndPlugin<IGitExtension> = {
IFileBrowserFactory,
IRenderMimeRegistry,
ISettingRegistry,
IDocumentManager,
IStatusBar
],
provides: IGitExtension,
@@ -54,6 +56,7 @@ async function activate(
factory: IFileBrowserFactory,
renderMime: IRenderMimeRegistry,
settingRegistry: ISettingRegistry,
docmanager: IDocumentManager,
statusBar: IStatusBar
): Promise<IGitExtension> {
let gitExtension: GitExtension | null = null;
@@ -110,7 +113,12 @@ async function activate(
return null;
}
// Create the Git model
gitExtension = new GitExtension(serverSettings.serverRoot, app, settings);
gitExtension = new GitExtension(
serverSettings.serverRoot,
app,
docmanager,
settings
);

// Whenever we restore the application, sync the Git extension path
Promise.all([app.restored, filebrowser.model.restored]).then(() => {
93 changes: 93 additions & 0 deletions src/model.ts
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import {
Poll,
ISettingRegistry
} from '@jupyterlab/coreutils';
import { IDocumentManager } from '@jupyterlab/docmanager';
import { ServerConnection } from '@jupyterlab/services';
import { CommandRegistry } from '@phosphor/commands';
import { JSONObject } from '@phosphor/coreutils';
@@ -31,11 +32,13 @@ export class GitExtension implements IGitExtension {
constructor(
serverRoot: string,
app: JupyterFrontEnd = null,
docmanager: IDocumentManager = null,
settings?: ISettingRegistry.ISettings
) {
const self = this;
this._serverRoot = serverRoot;
this._app = app;
this._docmanager = docmanager;
this._settings = settings || null;

let interval: number;
@@ -462,6 +465,19 @@ export class GitExtension implements IGitExtension {
const tid = this._addTask('git:checkout');
try {
response = await httpGitRequest('/git/checkout', 'POST', body);

if (response.ok) {
if (body.checkout_branch) {
const files = (
await this.changedFiles(this._currentBranch.name, body.branchname)
)['files'];
if (files) {
files.forEach(file => this._revertFile(file));
}
} else {
this._revertFile(options.filename);
}
}
} catch (err) {
throw new ServerConnection.NetworkError(err);
} finally {
@@ -471,6 +487,7 @@ export class GitExtension implements IGitExtension {
if (!response.ok) {
throw new ServerConnection.ResponseError(response, data.message);
}

if (body.checkout_branch) {
await this.refreshBranch();
this._headChanged.emit();
@@ -612,11 +629,17 @@ export class GitExtension implements IGitExtension {
return Promise.resolve(new Response(JSON.stringify(response)));
}
const tid = this._addTask('git:commit:revert');
const files = (await this.changedFiles(null, null, hash + '^!'))['files'];
try {
response = await httpGitRequest('/git/delete_commit', 'POST', {
commit_id: hash,
top_repo_path: path
});
if (response.ok && files) {
files.forEach(file => {
this._revertFile(file);
});
}
} catch (err) {
throw new ServerConnection.NetworkError(err);
} finally {
@@ -908,12 +931,29 @@ export class GitExtension implements IGitExtension {
return Promise.resolve(new Response(JSON.stringify(response)));
}
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 {
response = await httpGitRequest('/git/reset', 'POST', {
reset_all: filename === undefined,
filename: filename === undefined ? null : filename,
top_repo_path: path
});

if (response.ok) {
if (reset_all) {
if (files) {
files.forEach(file => {
this._revertFile(file);
});
}
} else {
this._revertFile(filename);
}
}
} catch (err) {
throw new ServerConnection.NetworkError(err);
} finally {
@@ -950,12 +990,20 @@ export class GitExtension implements IGitExtension {
};
return Promise.resolve(new Response(JSON.stringify(response)));
}
const files = (await this.changedFiles(null, null, hash))['files'];
const tid = this._addTask('git:reset:hard');
try {
response = await httpGitRequest('/git/reset_to_commit', 'POST', {
commit_id: hash,
top_repo_path: path
});
if (response.ok) {
if (files) {
files.forEach(file => {
this._revertFile(file);
});
}
}
} catch (err) {
throw new ServerConnection.NetworkError(err);
} finally {
@@ -1229,6 +1277,37 @@ export class GitExtension implements IGitExtension {
return Promise.resolve(response);
}

/**
* Get list of files changed between two commits or two branches
* @param base id of base commit or base branch for comparison
* @param remote id of remote commit or remote branch for comparison
* @param singleCommit id of a single commit
*
* @returns the names of the changed files
*/
async changedFiles(
base?: string,
remote?: string,
singleCommit?: string
): Promise<Git.IChangedFilesResult> {
try {
const response = await httpGitRequest('/git/changed_files', 'POST', {
current_path: this.pathRepository,
base: base,
remote: remote,
single_commit: singleCommit
});
if (!response.ok) {
return response.json().then((data: any) => {
throw new ServerConnection.ResponseError(response, data.message);
});
}
return response.json();
} catch (err) {
throw new ServerConnection.NetworkError(err);
}
}

/**
* Make request for a list of all git branches in the repository
* Retrieve a list of repository branches.
@@ -1347,12 +1426,26 @@ export class GitExtension implements IGitExtension {
return this._taskID;
}

/**
* 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
*
* @param path path to the file to be reverted
*/
private _revertFile(path: string): void {
const widget = this._docmanager.findWidget(this.getRelativeFilePath(path));
if (widget && !widget.context.model.dirty) {
widget.context.revert();
}
}

private _status: Git.IStatusFile[] = [];
private _pathRepository: string | null = null;
private _branches: Git.IBranch[];
private _currentBranch: Git.IBranch;
private _serverRoot: string;
private _app: JupyterFrontEnd | null;
private _docmanager: IDocumentManager | null;
private _diffProviders: { [key: string]: Git.IDiffCallback } = {};
private _isDisposed = false;
private _markerCache: Markers = new Markers(() => this._markChanged.emit());
9 changes: 9 additions & 0 deletions src/tokens.ts
Original file line number Diff line number Diff line change
@@ -448,6 +448,15 @@ export namespace Git {
files?: IStatusFileResult[];
}

/** Interface for changed_files request result
* lists the names of files that have differences between two commits
* or beween two branches, or that were changed by a single commit
*/
export interface IChangedFilesResult {
code: number;
files?: string[];
}

/** Interface for GitLog request result,
* has the info of a single past commit
*/
53 changes: 51 additions & 2 deletions tests/GitExtension.spec.tsx
Original file line number Diff line number Diff line change
@@ -10,6 +10,8 @@ describe('IGitExtension', () => {
const mockGit = git as jest.Mocked<typeof git>;
const fakeRoot = '/path/to/server';
let model: IGitExtension;
const docmanager = jest.mock('@jupyterlab/docmanager') as any;
docmanager.findWidget = jest.fn();
let mockResponses: {
[url: string]: {
body?: (request: Object) => string;
@@ -70,7 +72,7 @@ describe('IGitExtension', () => {
hasCommand: jest.fn().mockReturnValue(true)
}
};
model = new GitExtension(fakeRoot, app as any);
model = new GitExtension(fakeRoot, app as any, docmanager as any);
});

describe('#pathRepository', () => {
@@ -243,6 +245,45 @@ describe('IGitExtension', () => {
...mockResponses,
'/git/checkout': {
body: () => '{}'
},
'/git/branch': {
body: () =>
JSON.stringify({
code: 0,
branches: [
{
is_current_branch: true,
is_remote_branch: false,
name: 'master',
upstream: null,
top_commit: '52263564aac988a0888060becc3c76d1023e680f',
tag: null
},
{
is_current_branch: false,
is_remote_branch: false,
name: 'test-branch',
upstream: null,
top_commit: '52263564aac988a0888060becc3c76d1023e680f',
tag: null
}
],
current_branch: {
is_current_branch: true,
is_remote_branch: false,
name: 'master',
upstream: null,
top_commit: '52263564aac988a0888060becc3c76d1023e680f',
tag: null
}
})
},
'/git/changed_files': {
body: () =>
JSON.stringify({
code: 0,
files: ['']
})
}
};

@@ -255,7 +296,8 @@ describe('IGitExtension', () => {
}
});

await model.checkout({ branchname: 'dummy' });
await model.refreshBranch();
await model.checkout({ branchname: 'test-branch' });
await testSignal;
});
});
@@ -312,6 +354,13 @@ describe('IGitExtension', () => {
...mockResponses,
'/git/reset_to_commit': {
body: () => '{}'
},
'/git/changed_files': {
body: () =>
JSON.stringify({
code: 0,
files: ['made-up-file.md']
})
}
};

0 comments on commit 67d1896

Please sign in to comment.