Enhance Branch Creation UX and Refactor Issue Branch Management #6278

amrshadid · 2024-10-13T01:41:50Z

This PR introduces significant improvements to the branch creation and management process, focusing on enhancing the user experience and refactoring the existing code for better maintainability and performance. Key changes include:

Branch Selection UI Overhaul:
- Introduced a QuickPick interface for selecting branches, allowing users to choose from existing local/remote branches or create a new one.
- Visually highlighted branches related to the issue (e.g., branches containing the issue number).
- Provided a custom branch name option directly in the selection UI for flexible branch creation.

Some Related Issues:

#2243: Detect and use already-linked PR branches when starting work on an issue.
#4120: Detect branch names referencing an issue and automatically start working on them.
#4509: Improve transparency when creating and checking out branches for issues.

These issues are addressed by improving the branch selection experience, handling remote branches more seamlessly, and enhancing error feedback.

Testing and Verification:

Manual Testing:
- Verified that the QuickPick interface correctly displays existing branches and suggested branch names.
- Tested creation and checkout of local and remote branches.
- Confirmed proper upstream tracking for remote branches.
- Ensured that validation errors (e.g., invalid branch names) display appropriate messages.

- Implemented a `QuickPick` menu for smoother branch selection and creation process. - Highlighted suggested branch names with visual cues for better user experience. - Enhanced handling of remote branches, ensuring upstream is set automatically when needed. - Removed unnecessary methods (`offerNewBranch`, `getBranchTitle`) to simplify code and improve maintainability. - Improved branch name validation and error messages to provide more helpful feedback.

amrshadid · 2024-10-13T01:44:55Z

@microsoft-github-policy-service agree

nightscorn

cxc

nightscorn

ujghm

alexr00

Thanks for the PR! It looks like there are a lot of changes in here that aren't strictly related to the feature you're adding, which makes it slow to review. Could you please remove these changes? I've also left a few other comments.

src/issues/currentIssue.ts

- Refactored getBranchTitle() to correctly apply the ISSUE_BRANCH_TITLE setting by using variableSubstitution(). - Improved handling of the silent flag. - Cleaned up unrelated code changes.

amrshadid · 2024-10-19T13:32:33Z

@alexr00 Thanks for the feedback! I’ve addressed the specific comments and focused on ensuring the necessary changes related to the feature.

src/issues/currentIssue.ts

alexr00

@amrshadid, thanks for your patience! I've left some suggestions and comments.

alexr00 · 2024-12-09T10:10:02Z

src/issues/currentIssue.ts

@@ -130,19 +129,30 @@ export class CurrentIssue extends Disposable {
 		return undefined;
 	}

-	private async createOrCheckoutBranch(branch: string): Promise<boolean> {
+	private async createOrCheckoutBranch(branch: string, silent: boolean): Promise<boolean> {


Does this really need a silent? It seems like something that would always be useful to know if it failed.

alexr00 · 2024-12-09T10:11:54Z

src/issues/currentIssue.ts

-	private showBranchNameError(error: string) {
-		const editSetting = `Edit Setting`;
-		vscode.window.showErrorMessage(error, editSetting).then(result => {
+	private showBranchNameError(error: string, silent: boolean) {


Same for this. I think if the branch name is bad then we should always show an error.

alexr00 · 2024-12-09T10:35:22Z

src/issues/currentIssue.ts

@@ -130,19 +129,30 @@ export class CurrentIssue extends Disposable {
 		return undefined;
 	}



Checking that the branch name starts with origin/ isn't enough, as not all remotes will be named origin.

Suggested change

private getRemoteBranchPrefix(branch: string): string | undefined {

const remote = this.manager.repository.state.remotes.find((remote) => {

return branch.startsWith(`${remote.name}/`);

});

return remote ? `${remote.name}/` : undefined;

}

alexr00 · 2024-12-09T10:35:24Z

src/issues/currentIssue.ts

+			const isRemoteBranch = branch.startsWith('origin/');
+			if (isRemoteBranch) {
+				localBranchName = branch.substring('origin/'.length);


Suggested change

const isRemoteBranch = branch.startsWith('origin/');

if (isRemoteBranch) {

localBranchName = branch.substring('origin/'.length);

const remoteBranchPrefix = this.getRemoteBranchPrefix(branch);

if (remoteBranchPrefix) {

localBranchName = branch.substring(remoteBranchPrefix.length);

alexr00 · 2024-12-09T10:35:25Z

src/issues/currentIssue.ts

+			const localBranch = await this.getBranch(localBranchName);
+			if (localBranch) {
+				await this.manager.repository.checkout(localBranchName);
+			} else if (isRemoteBranch) {


Suggested change

} else if (isRemoteBranch) {

} else if (remoteBranchPrefix) {

alexr00 · 2024-12-09T10:38:48Z

src/issues/currentIssue.ts

+		);
+
+		// Show QuickPick for branch selection
+		const quickPick = vscode.window.createQuickPick<vscode.QuickPickItem>();


Can you just use showQuickPick here instead of the more complicated createQuickPick?

alexr00 · 2024-12-09T10:40:02Z

src/issues/currentIssue.ts

+
+		// Create QuickPick items
+		const branchItems: vscode.QuickPickItem[] = [
+			{ label: 'Suggested branch', kind: vscode.QuickPickItemKind.Separator },


All the strings should be localized with vscode.l10n.t

alexr00 · 2024-12-09T10:40:39Z

src/issues/currentIssue.ts

+
+		branchItems.push(
+			{ label: 'Custom branch name:', kind: vscode.QuickPickItemKind.Separator },
+			{ label: '$(pencil) Enter a custom branch name...', description: 'Choose this to type your own branch name' }


Since this string is used again further down, it would be better to move it into a const, especially after it's been localized.

alexr00 · 2024-12-09T10:42:43Z

src/issues/currentIssue.ts

+
+			this._branchName = customBranchName.trim();
+		} else {
+			this._branchName = selectedBranch.label.replace(/^\$\([^\)]+\)\s*/, '').trim();


Instead of futzing with the label after the branch has been picked, you can extend QuickPickItem and add a new property to it to track the actual branch name associated with the quick pick item.

alexr00 · 2024-12-09T10:51:37Z

src/issues/currentIssue.ts

+			{ label: '$(pencil) Enter a custom branch name...', description: 'Choose this to type your own branch name' }
+		);
+
+		// Show QuickPick for branch selection


We should only show the quick pick if createBranchConfig === 'prompt.

amrshadid added 2 commits October 16, 2024 12:25

Merge branch 'main' into feature/branch-search-improvements

6808386

Merge branch 'main' into feature/branch-search-improvements

b59e5a9

nightscorn reviewed Oct 17, 2024

View reviewed changes

amrshadid added 2 commits October 17, 2024 12:38

Merge branch 'main' into feature/branch-search-improvements

aff10c4

Merge branch 'main' into feature/branch-search-improvements

fb62463

alexr00 requested changes Oct 18, 2024

View reviewed changes

src/issues/currentIssue.ts Show resolved Hide resolved

src/issues/currentIssue.ts Show resolved Hide resolved

amrshadid and others added 2 commits October 18, 2024 17:38

Merge branch 'main' into feature/branch-search-improvements

409a968

fix: Ensure proper handling of ISSUE_BRANCH_TITLE and silent flag

30a9a0c

- Refactored getBranchTitle() to correctly apply the ISSUE_BRANCH_TITLE setting by using variableSubstitution(). - Improved handling of the silent flag. - Cleaned up unrelated code changes.

amrshadid added 7 commits October 21, 2024 23:47

Merge branch 'main' into feature/branch-search-improvements

cefb91f

Merge branch 'main' into feature/branch-search-improvements

212c6e0

Merge branch 'main' into feature/branch-search-improvements

1c807cc

Merge branch 'main' into feature/branch-search-improvements

ba091f0

Merge branch 'main' into feature/branch-search-improvements

96d5658

Merge branch 'main' into feature/branch-search-improvements

b25d5f8

Merge branch 'main' into feature/branch-search-improvements

139b258

amrshadid requested a review from alexr00 October 29, 2024 10:21

amrshadid added 2 commits October 29, 2024 14:13

Merge branch 'main' into feature/branch-search-improvements

41f1c32

Merge branch 'main' into feature/branch-search-improvements

e3cf4dc

amrshadid requested a review from nightscorn October 29, 2024 22:21

nightscorn approved these changes Oct 29, 2024

View reviewed changes

amrshadid added 4 commits October 30, 2024 16:39

Merge branch 'main' into feature/branch-search-improvements

13ce6b0

Merge branch 'main' into feature/branch-search-improvements

e4f989d

Merge branch 'main' into feature/branch-search-improvements

c1eb15d

Merge branch 'main' into feature/branch-search-improvements

44903c2

amrshadid added 2 commits November 13, 2024 19:06

Merge branch 'main' into feature/branch-search-improvements

2844546

Merge branch 'main' into feature/branch-search-improvements

7aba54b

venkateshpat reviewed Nov 27, 2024

View reviewed changes

src/issues/currentIssue.ts Show resolved Hide resolved

Merge branch 'main' into feature/branch-search-improvements

a16f2ee

alexr00 requested changes Dec 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Branch Creation UX and Refactor Issue Branch Management #6278

Enhance Branch Creation UX and Refactor Issue Branch Management #6278

amrshadid commented Oct 13, 2024 •

edited

Loading

amrshadid commented Oct 13, 2024

nightscorn left a comment

nightscorn left a comment

alexr00 left a comment

amrshadid commented Oct 19, 2024

alexr00 left a comment

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

alexr00 Dec 9, 2024

		@@ -130,19 +129,30 @@ export class CurrentIssue extends Disposable {
		return undefined;
		}

+	private getRemoteBranchPrefix(branch: string): string | undefined {
+		const remote = this.manager.repository.state.remotes.find((remote) => {
+			return branch.startsWith(`${remote.name}/`);
+		});
+		return remote ? `${remote.name}/` : undefined;
+	}

	} else if (isRemoteBranch) {
	} else if (remoteBranchPrefix) {

Enhance Branch Creation UX and Refactor Issue Branch Management #6278

Are you sure you want to change the base?

Enhance Branch Creation UX and Refactor Issue Branch Management #6278

Conversation

amrshadid commented Oct 13, 2024 • edited Loading

amrshadid commented Oct 13, 2024

nightscorn left a comment

Choose a reason for hiding this comment

nightscorn left a comment

Choose a reason for hiding this comment

alexr00 left a comment

Choose a reason for hiding this comment

amrshadid commented Oct 19, 2024

alexr00 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amrshadid commented Oct 13, 2024 •

edited

Loading