From 3f86b16d821b3a7786cefbd8d90c197b5a1dab37 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 3 Nov 2023 01:20:45 -0400 Subject: [PATCH] Fixes #2997 ensures push to commit works properly Adds support to "publish" to commit as well --- CHANGELOG.md | 2 + src/commands/git/push.ts | 58 +++++++++++++--------- src/env/node/git/git.ts | 4 ++ src/env/node/git/localGitProvider.ts | 94 ++++++++++++++++++++++++++---------- src/git/gitProvider.ts | 4 +- src/git/gitProviderService.ts | 2 +- src/git/models/repository.ts | 21 +++----- src/plus/github/githubGitProvider.ts | 6 ++- 8 files changed, 126 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f1c066..5041b0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Adds a `gitlens.sortRepositoriesBy` setting to specify how repositories are sorted in quick pick menus and viewsby Aidos Kanapyanov ([@aidoskanapyanov](https://github.com/aidoskanapyanov)) - Adds a _[Show|Hide] Merge Commits_ toggle to the Commits\_ view — closes [#1399](https://github.com/gitkraken/vscode-gitlens/issues/1399) thanks to [PR #1540](https://github.com/gitkraken/vscode-gitlens/pull/1540) by Shashank Shastri ([@Shashank-Shastri](https://github.com/Shashank-Shastri)) - Adds a _Filter Commits by Author..._ commands to the _Commits_ view and comparisons context menus to filter commits in the _Commits_ view by specific authors +- Adds ability to publish to a remote branch to a specific commit using the _Push to Commit_ command - Adds an _Open Comparison on Remote_ command to comparisons in views - Adds a _Share > Copy Link to Repository_ command on branches in the views - Adds _Share > Copy Link to Branch_ and _Share > Copy Link to Repository_ commands on the current branch status in the _Commits_ view @@ -41,6 +42,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### Fixed +- Fixes [#2997](https://github.com/gitkraken/vscode-gitlens/issues/2997) - "push to commit" pushes everything instead of up to the selected commit - Fixes [#2615](https://github.com/gitkraken/vscode-gitlens/issues/2615) - Source Control views disappear after opening a file beyond a symbolic link - Fixes [#2443](https://github.com/gitkraken/vscode-gitlens/issues/2443) - UNC-PATH: File History changes not displaying any changes when open - Fixes [#2625](https://github.com/gitkraken/vscode-gitlens/issues/2625) - full issue ref has escape characters that break hover links diff --git a/src/commands/git/push.ts b/src/commands/git/push.ts index 71eeed7..b7bba98 100644 --- a/src/commands/git/push.ts +++ b/src/commands/git/push.ts @@ -69,8 +69,6 @@ export class PushGitCommand extends QuickCommand { execute(state: State) { const index = state.flags.indexOf('--set-upstream'); if (index !== -1) { - if (!isBranchReference(state.reference)) return Promise.resolve(); - return this.container.git.pushAll(state.repos, { force: false, publish: { remote: state.flags[index + 1] }, @@ -192,12 +190,13 @@ export class PushGitCommand extends QuickCommand { if (isBranchReference(state.reference)) { if (state.reference.remote) { step = this.createConfirmStep( - appendReposToTitle(`Confirm ${context.title}`, state, context), + appendReposToTitle(context.title, state, context), [], createDirectiveQuickPickItem(Directive.Cancel, true, { - label: `Cancel ${this.title}`, - detail: 'Cannot push remote branch', + label: 'OK', + detail: 'Cannot push a remote branch', }), + { placeholder: 'Cannot push a remote branch' }, ); } else { const branch = await repo.getBranch(state.reference.name); @@ -225,13 +224,13 @@ export class PushGitCommand extends QuickCommand { ); } else { step = this.createConfirmStep( - appendReposToTitle('Confirm Publish', state, context), + appendReposToTitle('Publish', state, context), [], createDirectiveQuickPickItem(Directive.Cancel, true, { - label: 'Cancel Publish', - detail: 'Cannot publish; No remotes found', + label: 'OK', + detail: 'No remotes found', }), - { placeholder: 'Confirm Publish' }, + { placeholder: 'Cannot publish; No remotes found' }, ); } } else if (branch != null && branch?.state.behind > 0) { @@ -286,12 +285,13 @@ export class PushGitCommand extends QuickCommand { ]); } else { step = this.createConfirmStep( - appendReposToTitle(`Confirm ${context.title}`, state, context), + appendReposToTitle(context.title, state, context), [], createDirectiveQuickPickItem(Directive.Cancel, true, { - label: `Cancel ${this.title}`, + label: 'OK', detail: 'No commits found to push', }), + { placeholder: 'Nothing to push; No commits found to push' }, ); } } @@ -307,8 +307,17 @@ export class PushGitCommand extends QuickCommand { }; if (status?.state.ahead === 0) { - if (state.reference == null && status.upstream == null) { - state.reference = branch; + if (!isBranchReference(state.reference) && status.upstream == null) { + let pushDetails; + + if (state.reference != null) { + pushDetails = ` up to and including ${getReferenceLabel(state.reference, { + label: false, + })}`; + } else { + state.reference = branch; + pushDetails = ''; + } for (const remote of await repo.getRemotes()) { items.push( @@ -317,7 +326,9 @@ export class PushGitCommand extends QuickCommand { ['--set-upstream', remote.name, status.branch], { label: `Publish ${branch.name} to ${remote.name}`, - detail: `Will publish ${getReferenceLabel(branch)} to ${remote.name}`, + detail: `Will publish ${getReferenceLabel(branch)}${pushDetails} to ${ + remote.name + }`, }, ), ); @@ -333,24 +344,27 @@ export class PushGitCommand extends QuickCommand { ); } else if (status.upstream == null) { step = this.createConfirmStep( - appendReposToTitle('Confirm Publish', state, context), + appendReposToTitle('Publish', state, context), [], createDirectiveQuickPickItem(Directive.Cancel, true, { - label: 'Cancel Publish', - detail: 'Cannot publish; No remotes found', + label: 'OK', + detail: 'No remotes found', }), - { placeholder: 'Confirm Publish' }, + { placeholder: 'Cannot publish; No remotes found' }, ); } else { step = this.createConfirmStep( - appendReposToTitle('Confirm Push', state, context), + appendReposToTitle(context.title, state, context), [], createDirectiveQuickPickItem(Directive.Cancel, true, { - label: `Cancel ${this.title}`, - detail: `Cannot push; No commits ahead of ${getRemoteNameFromBranchName( + label: 'OK', + detail: `No commits ahead of ${getRemoteNameFromBranchName(status.upstream)}`, + }), + { + placeholder: `Nothing to push; No commits ahead of ${getRemoteNameFromBranchName( status.upstream, )}`, - }), + }, ); } } else { diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 01054a6..bd92047 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -478,6 +478,10 @@ export class Git { } } + branch__set_upstream(repoPath: string, branch: string, remote: string, remoteBranch: string) { + return this.git({ cwd: repoPath }, 'branch', '--set-upstream-to', `${remote}/${remoteBranch}`, branch); + } + branchOrTag__containsOrPointsAt( repoPath: string, ref: string, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 147179f..7703ab7 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -22,6 +22,7 @@ import { GitSearchError, PullError, PushError, + PushErrorReason, StashApplyError, StashApplyErrorReason, WorktreeCreateError, @@ -78,7 +79,7 @@ import type { import type { GitLog } from '../../../git/models/log'; import type { GitMergeStatus } from '../../../git/models/merge'; import type { GitRebaseStatus } from '../../../git/models/rebase'; -import type { GitBranchReference, GitTagReference } from '../../../git/models/reference'; +import type { GitBranchReference, GitReference, GitTagReference } from '../../../git/models/reference'; import { createReference, getBranchTrackingWithoutRemote, @@ -1213,11 +1214,9 @@ export class LocalGitProvider implements GitProvider, Disposable { this.container.events.fire('git:cache:reset', { repoPath: repoPath }); } catch (ex) { Logger.error(ex, scope); - if (FetchError.is(ex)) { - void window.showErrorMessage(ex.message); - } else { - throw ex; - } + if (!FetchError.is(ex)) throw ex; + + void window.showErrorMessage(ex.message); } } @@ -1225,19 +1224,56 @@ export class LocalGitProvider implements GitProvider, Disposable { @log() async push( repoPath: string, - options?: { branch?: GitBranchReference; force?: boolean; publish?: { remote: string } }, + options?: { reference?: GitReference; force?: boolean; publish?: { remote: string } }, ): Promise { const scope = getLogScope(); - let branch = options?.branch; - if (!isBranchReference(branch)) { - branch = await this.getBranch(repoPath); - if (branch == null) return undefined; + let branchName: string; + let remoteName: string | undefined; + let upstreamName: string | undefined; + let setUpstream: + | { + branch: string; + remote: string; + remoteBranch: string; + } + | undefined; + + if (isBranchReference(options?.reference)) { + if (options.publish != null) { + branchName = options.reference.name; + remoteName = options.publish.remote; + upstreamName = getBranchTrackingWithoutRemote(options.reference); + } else { + [branchName, remoteName] = getBranchNameAndRemote(options.reference); + upstreamName = undefined; + } + } else { + const branch = await this.getBranch(repoPath); + if (branch == null) return; + + branchName = + options?.reference != null + ? `${options.reference.ref}:${ + options?.publish != null ? 'refs/heads/' : '' + }${branch.getNameWithoutRemote()}` + : branch.name; + remoteName = branch.getRemoteName() ?? options?.publish?.remote; + upstreamName = options?.reference == null && options?.publish != null ? branch.name : undefined; + + // Git can't setup remote tracking when publishing a new branch to a specific commit, so we'll need to do it after the push + if (options?.publish?.remote != null && options?.reference != null) { + setUpstream = { + branch: branch.getNameWithoutRemote(), + remote: remoteName!, + remoteBranch: branch.getNameWithoutRemote(), + }; + } } - const [branchName, remoteName] = getBranchNameAndRemote(branch); - if (options?.publish == null && remoteName == null && branch.upstream == null) { - return undefined; + if (options?.publish == null && remoteName == null && upstreamName == null) { + debugger; + throw new PushError(PushErrorReason.Other); } let forceOpts: PushForceOptions | undefined; @@ -1259,20 +1295,28 @@ export class LocalGitProvider implements GitProvider, Disposable { try { await this.git.push(repoPath, { branch: branchName, - remote: options?.publish ? options.publish.remote : remoteName, - upstream: getBranchTrackingWithoutRemote(branch), + remote: remoteName, + upstream: upstreamName, force: forceOpts, publish: options?.publish != null, }); + // Since Git can't setup remote tracking when publishing a new branch to a specific commit, do it now + if (setUpstream != null) { + await this.git.branch__set_upstream( + repoPath, + setUpstream.branch, + setUpstream.remote, + setUpstream.remoteBranch, + ); + } + this.container.events.fire('git:cache:reset', { repoPath: repoPath }); } catch (ex) { Logger.error(ex, scope); - if (PushError.is(ex)) { - void window.showErrorMessage(ex.message); - } else { - throw ex; - } + if (!PushError.is(ex)) throw ex; + + void window.showErrorMessage(ex.message); } } @@ -1304,11 +1348,9 @@ export class LocalGitProvider implements GitProvider, Disposable { this.container.events.fire('git:cache:reset', { repoPath: repoPath }); } catch (ex) { Logger.error(ex, scope); - if (PullError.is(ex)) { - void window.showErrorMessage(ex.message); - } else { - throw ex; - } + if (!PullError.is(ex)) throw ex; + + void window.showErrorMessage(ex.message); } } diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index f2f2cf4..570afa5 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -13,7 +13,7 @@ import type { GitGraph } from './models/graph'; import type { GitLog } from './models/log'; import type { GitMergeStatus } from './models/merge'; import type { GitRebaseStatus } from './models/rebase'; -import type { GitBranchReference } from './models/reference'; +import type { GitBranchReference, GitReference } from './models/reference'; import type { GitReflog } from './models/reflog'; import type { GitRemote } from './models/remote'; import type { Repository, RepositoryChangeEvent } from './models/repository'; @@ -166,7 +166,7 @@ export interface GitProvider extends Disposable { push( repoPath: string, options?: { - branch?: GitBranchReference | undefined; + reference?: GitReference | undefined; force?: boolean | undefined; publish?: { remote: string }; }, diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index b8a324d..a7a7907 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1400,7 +1400,7 @@ export class GitProviderService implements Disposable { @log() push( repoPath: string | Uri, - options?: { branch?: GitBranchReference; force?: boolean; publish?: { remote: string } }, + options?: { reference?: GitReference; force?: boolean; publish?: { remote: string } }, ): Promise { const { provider, path } = this.getProvider(repoPath); return provider.push(path, options); diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index 95657e8..fa012ab 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -891,9 +891,7 @@ export class Repository implements Disposable { force?: boolean; progress?: boolean; reference?: GitReference; - publish?: { - remote: string; - }; + publish?: { remote: string }; }) { const { progress, ...opts } = { progress: true, ...options }; if (!progress) return this.pushCore(opts); @@ -909,21 +907,18 @@ export class Repository implements Disposable { ); } - private async pushCore(options?: { - force?: boolean; - reference?: GitReference; - publish?: { - remote: string; - }; - }) { + private async pushCore(options?: { force?: boolean; reference?: GitReference; publish?: { remote: string } }) { try { if (configuration.get('experimental.nativeGit')) { - const branch = await this.getBranch(options?.reference?.name); await this.container.git.push(this.uri, { + reference: options?.reference, force: options?.force, - branch: isBranchReference(options?.reference) ? options?.reference : branch, - ...(options?.publish && { publish: options.publish }), + publish: options?.publish, }); + + if (isBranchReference(options?.reference) && options?.publish != null) { + void this.showCreatePullRequestPrompt(options.publish.remote, options.reference); + } } else if (isBranchReference(options?.reference)) { const repo = await this.container.git.getOrOpenScmRepository(this.uri); if (repo == null) return; diff --git a/src/plus/github/githubGitProvider.ts b/src/plus/github/githubGitProvider.ts index cc12344..5ee1466 100644 --- a/src/plus/github/githubGitProvider.ts +++ b/src/plus/github/githubGitProvider.ts @@ -495,7 +495,11 @@ export class GitHubGitProvider implements GitProvider, Disposable { @log() async push( _repoPath: string, - _options?: { branch?: GitBranchReference; force?: boolean; publish?: { remote: string } }, + _options?: { + reference?: GitReference; + force?: boolean; + publish?: { remote: string }; + }, ): Promise {} @gate()