From f35465ae8692229ffb8def6d879a5f1f46f4328c Mon Sep 17 00:00:00 2001 From: Eldar Yusupov Date: Thu, 10 Aug 2023 12:25:46 +0300 Subject: [PATCH] Add option to ask user identity on every commit (#1251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add option to ask user identity on every commit * Override author * Set author in config when not set even with prompt on every commit * Fix test * Apply suggestions from code review Co-authored-by: Frédéric Collonval * Cleanup the code * Fix formatting * Improve author form layout * Fix linter --------- Co-authored-by: Frédéric Collonval Co-authored-by: Frédéric Collonval --- README.md | 1 + jupyterlab_git/git.py | 4 +- jupyterlab_git/handlers.py | 5 +- schema/plugin.json | 6 ++ .../test-components/GitPanel.spec.tsx | 34 +++++++++-- src/components/GitPanel.tsx | 61 +++++++++++++------ src/model.ts | 23 ++++++- src/widgets/AuthorBox.ts | 38 ++++++++---- 8 files changed, 133 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 84c26cb4b..e48fa92f0 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,7 @@ Once installed, extension behavior can be modified via the following settings wh - **displayStatus**: display Git extension status updates in the JupyterLab status bar. If `true`, the extension displays status updates in the JupyterLab status bar, such as when pulling and pushing changes, switching branches, and polling for changes. Depending on the level of extension activity, some users may find the status updates distracting. In which case, setting this to `false` should reduce visual noise. - **doubleClickDiff**: double click a file in the Git extension panel to open a diff of the file instead of opening the file for editing. - **historyCount**: number of commits shown in the history log, beginning with the most recent. Displaying a larger number of commits can lead to performance degradation, so use caution when modifying this setting. +- **promptUserIdentity**: Whether to prompt for user name and email on every commit. - **refreshIfHidden**: whether to refresh even if the Git tab is hidden; default to `false` (i.e. refresh is turned off if the Git tab is hidden). - **refreshInterval**: number of milliseconds between polling the file system for changes. In order to ensure that the UI correctly displays the current repository status, the extension must poll the file system for changes. Longer polling times increase the likelihood that the UI does not reflect the current status; however, longer polling times also incur less performance overhead. - **simpleStaging**: enable a simplified concept of staging. When this setting is `true`, all files with changes are automatically staged. When we develop in JupyterLab, we often only care about what files have changed (in the broadest sense) and don't need to distinguish between "tracked" and "untracked" files. Accordingly, this setting allows us to simplify the visual presentation of changes, which is especially useful for those less acquainted with Git. diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 336f37bae..b2f07d893 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -1156,13 +1156,15 @@ async def merge(self, branch: str, path: str) -> dict: return {"code": code, "command": " ".join(cmd), "message": error} return {"code": code, "message": output.strip()} - async def commit(self, commit_msg, amend, path): + async def commit(self, commit_msg, amend, path, author=None): """ Execute git commit command & return the result. If the amend argument is true, amend the commit instead of creating a new one. """ cmd = ["git", "commit"] + if author: + cmd.extend(["--author", author]) if amend: cmd.extend(["--amend", "--no-edit"]) else: diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py index 3cb3c57e4..5cdf6f906 100644 --- a/jupyterlab_git/handlers.py +++ b/jupyterlab_git/handlers.py @@ -561,7 +561,10 @@ async def post(self, path: str = ""): data = self.get_json_body() commit_msg = data["commit_msg"] amend = data.get("amend", False) - body = await self.git.commit(commit_msg, amend, self.url2localpath(path)) + author = data.get("author") + body = await self.git.commit( + commit_msg, amend, self.url2localpath(path), author + ) if body["code"] != 0: self.set_status(500) diff --git a/schema/plugin.json b/schema/plugin.json index 848073251..3b40fda7b 100644 --- a/schema/plugin.json +++ b/schema/plugin.json @@ -65,6 +65,12 @@ "description": "Whether to trigger or not a push for each commit.", "default": false }, + "promptUserIdentity": { + "type": "boolean", + "title": "Prompt user identity on commit", + "description": "Whether to prompt for user name and email for each commit.", + "default": false + }, "openFilesBehindWarning": { "type": "boolean", "title": "Open files behind warning", diff --git a/src/__tests__/test-components/GitPanel.spec.tsx b/src/__tests__/test-components/GitPanel.spec.tsx index 1b1a3b36e..04b363df2 100644 --- a/src/__tests__/test-components/GitPanel.spec.tsx +++ b/src/__tests__/test-components/GitPanel.spec.tsx @@ -40,14 +40,15 @@ const mockedResponses: { * @private * @returns mock settings */ -function MockSettings(commitAndPush = true) { +function MockSettings(commitAndPush = true, promptUserIdentity = false) { return { changed: { connect: () => true, disconnect: () => true }, composite: { - commitAndPush + commitAndPush, + promptUserIdentity } }; } @@ -161,7 +162,9 @@ describe('GitPanel', () => { expect(configSpy).toHaveBeenCalledTimes(1); expect(commitSpy).toHaveBeenCalledTimes(1); expect(commitSpy).toHaveBeenCalledWith( - commitSummary + '\n\n' + commitDescription + '\n' + commitSummary + '\n\n' + commitDescription + '\n', + false, + null ); // Only erase commit message upon success @@ -175,17 +178,38 @@ describe('GitPanel', () => { expect(commitSpy).not.toHaveBeenCalled(); }); + it('should prompt for user identity if explicitly configured', async () => { + configSpy.mockResolvedValue({ options: commitUser }); + + props.settings = MockSettings(false, true) as any; + const panelWrapper = shallow(); + panel = panelWrapper.instance(); + + mockUtils.showDialog.mockResolvedValue(dialogValue); + + panel.setState({ commitSummary }); + + await panel.commitFiles(); + expect(configSpy).toHaveBeenCalledTimes(1); + expect(configSpy.mock.calls[0]).toHaveLength(0); + + const author = `${commitUser['user.name']} <${commitUser['user.email']}>`; + expect(commitSpy).toHaveBeenCalledTimes(1); + expect(commitSpy).toHaveBeenCalledWith(commitSummary, false, author); + }); + it('should prompt for user identity if user.name is not set', async () => { configSpy.mockImplementation(mockConfigImplementation('user.email')); mockUtils.showDialog.mockResolvedValue(dialogValue); panel.setState({ commitSummary }); + await panel.commitFiles(); expect(configSpy).toHaveBeenCalledTimes(2); expect(configSpy.mock.calls[0]).toHaveLength(0); expect(configSpy.mock.calls[1]).toEqual([commitUser]); expect(commitSpy).toHaveBeenCalledTimes(1); - expect(commitSpy).toHaveBeenCalledWith(commitSummary); + expect(commitSpy).toHaveBeenCalledWith(commitSummary, false, null); }); it('should prompt for user identity if user.email is not set', async () => { @@ -198,7 +222,7 @@ describe('GitPanel', () => { expect(configSpy.mock.calls[0]).toHaveLength(0); expect(configSpy.mock.calls[1]).toEqual([commitUser]); expect(commitSpy).toHaveBeenCalledTimes(1); - expect(commitSpy).toHaveBeenCalledWith(commitSummary); + expect(commitSpy).toHaveBeenCalledWith(commitSummary, false, null); }); it('should not commit if no user identity is set and the user rejects the dialog', async () => { diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index e00eac6f8..874d98a31 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -770,7 +770,7 @@ export class GitPanel extends React.Component { }; try { - await this._hasIdentity(this.props.model.pathRepository); + const author = await this._hasIdentity(this.props.model.pathRepository); this.props.logger.log({ level: Level.RUNNING, @@ -778,9 +778,9 @@ export class GitPanel extends React.Component { }); if (this.state.commitAmend) { - await this.props.model.commit(null, true); + await this.props.model.commit(null, true, author); } else { - await this.props.model.commit(message); + await this.props.model.commit(message, false, author); } this.props.logger.log({ @@ -807,19 +807,38 @@ export class GitPanel extends React.Component { * * @param path - repository path */ - private async _hasIdentity(path: string): Promise { - // If the repository path changes, check the user identity - if (path !== this._previousRepoPath) { + private async _hasIdentity(path: string): Promise { + // If the repository path changes or explicitly configured, check the user identity + if ( + path !== this._previousRepoPath || + this.props.settings.composite['promptUserIdentity'] + ) { try { - const data: JSONObject = (await this.props.model.config()) as any; - const options: JSONObject = data['options'] as JSONObject; - const keys = Object.keys(options); + let userOrEmailNotSet = false; + let author: Git.IIdentity; + let authorOverride: string | null = null; + + if (this.props.model.lastAuthor === null) { + const data: JSONObject = (await this.props.model.config()) as any; + const options: JSONObject = data['options'] as JSONObject; + + author = { + name: (options['user.name'] as string) || '', + email: (options['user.email'] as string) || '' + }; + userOrEmailNotSet = !author.name || !author.email; + } else { + author = this.props.model.lastAuthor; + } - // If the user name or e-mail is unknown, ask the user to set it - if (keys.indexOf('user.name') < 0 || keys.indexOf('user.email') < 0) { + // If explicitly configured or the user name or e-mail is unknown, ask the user to set it + if ( + this.props.settings.composite['promptUserIdentity'] || + userOrEmailNotSet + ) { const result = await showDialog({ title: this.props.trans.__('Who is committing?'), - body: new GitAuthorForm() + body: new GitAuthorForm({ author, trans: this.props.trans }) }); if (!result.button.accept) { @@ -828,13 +847,21 @@ export class GitPanel extends React.Component { ); } - const { name, email } = result.value; - await this.props.model.config({ - 'user.name': name, - 'user.email': email - }); + author = result.value; + if (userOrEmailNotSet) { + await this.props.model.config({ + 'user.name': author.name, + 'user.email': author.email + }); + } + this.props.model.lastAuthor = author; + + if (this.props.settings.composite['promptUserIdentity']) { + authorOverride = `${author.name} <${author.email}>`; + } } this._previousRepoPath = path; + return authorOverride; } catch (error) { if (error instanceof Git.GitResponseError) { throw error; diff --git a/src/model.ts b/src/model.ts index 84e9144e5..f5bb977e3 100644 --- a/src/model.ts +++ b/src/model.ts @@ -198,6 +198,18 @@ export class GitExtension implements IGitExtension { } } + /** + * Last author + * + */ + get lastAuthor(): Git.IIdentity | null { + return this._lastAuthor; + } + + set lastAuthor(lastAuthor: Git.IIdentity) { + this._lastAuthor = lastAuthor; + } + /** * Git repository status */ @@ -670,18 +682,24 @@ export class GitExtension implements IGitExtension { * * @param message - commit message * @param amend - whether this is an amend commit + * @param author - override the commit author specified in config * @returns promise which resolves upon committing file changes * * @throws {Git.NotInRepository} If the current path is not a Git repository * @throws {Git.GitResponseError} If the server response is not ok * @throws {ServerConnection.NetworkError} If the request cannot be made */ - async commit(message?: string, amend = false): Promise { + async commit( + message?: string, + amend = false, + author?: string + ): Promise { const path = await this._getPathRepository(); await this._taskHandler.execute('git:commit:create', async () => { await requestAPI(URLExt.join(path, 'commit'), 'POST', { commit_msg: message, - amend: amend + amend: amend, + author: author }); }); await this.refresh(); @@ -2118,6 +2136,7 @@ export class GitExtension implements IGitExtension { private _selectedHistoryFile: Git.IStatusFile | null = null; private _hasDirtyFiles = false; private _credentialsRequired = false; + private _lastAuthor: Git.IIdentity | null = null; // Configurable private _statusForDirtyState: Git.Status[] = ['staged', 'partially-staged']; diff --git a/src/widgets/AuthorBox.ts b/src/widgets/AuthorBox.ts index ae9c10590..92c5694fe 100644 --- a/src/widgets/AuthorBox.ts +++ b/src/widgets/AuthorBox.ts @@ -1,4 +1,5 @@ import { Dialog } from '@jupyterlab/apputils'; +import { TranslationBundle } from '@jupyterlab/translation'; import { Widget } from '@lumino/widgets'; import { Git } from '../tokens'; @@ -9,26 +10,37 @@ export class GitAuthorForm extends Widget implements Dialog.IBodyWidget { - constructor() { + constructor({ + author, + trans + }: { + author: Git.IIdentity; + trans: TranslationBundle; + }) { super(); - this.node.appendChild(this.createBody()); + this._populateForm(author, trans); } - private createBody(): HTMLElement { - const node = document.createElement('div'); - const text = document.createElement('span'); - this._name = document.createElement('input'); - this._email = document.createElement('input'); + private _populateForm( + author: Git.IIdentity, + trans?: TranslationBundle + ): void { + const nameLabel = document.createElement('label'); + nameLabel.textContent = trans.__('Committer name:'); + const emailLabel = document.createElement('label'); + emailLabel.textContent = trans.__('Committer email:'); - node.className = 'jp-RedirectForm'; - text.textContent = 'Enter your name and email for commit'; + this._name = nameLabel.appendChild(document.createElement('input')); + this._email = emailLabel.appendChild(document.createElement('input')); this._name.placeholder = 'Name'; + this._email.type = 'text'; this._email.placeholder = 'Email'; + this._email.type = 'email'; + this._name.value = author.name; + this._email.value = author.email; - node.appendChild(text); - node.appendChild(this._name); - node.appendChild(this._email); - return node; + this.node.appendChild(nameLabel); + this.node.appendChild(emailLabel); } /**