Correctly handle draft status when editing an existing pull request

- Keep current draft status by default
- When draft status changes, use gh pr ready (--undo) to update it

Fixes #175

Author: annervisser

commands/pick/pick-command.ts

@@ -2,7 +2,7 @@ import { confirmSettings } from './steps/confirm-settings.ts';
 import { runCherryPick } from './steps/git-pick.ts';
 import * as log from '@std/log';
 import { Git } from '../../lib/git/git.ts';
-import { GH } from '../../lib/github/gh.ts';
+import { GH, type PullRequest } from '../../lib/github/gh.ts';
 import { Gum } from '../../lib/gum/gum.ts';
 import {
 	assertValidBranchName,
@@ -113,10 +113,10 @@ export const pickCommand = new Command()
 		await assertValidBranchName(branchName);
 
 		const remoteBranchExists = await Git.doesBranchExist(`${options.pushRemote}/${branchName}`);
-		let doesBranchHavePullRequest: Promise<boolean> | undefined;
+		let existingPullRequestPromise: Promise<PullRequest | null> | null = null;
 		if (options.pr && remoteBranchExists) {
 			// Start this check early to save some waiting time
-			doesBranchHavePullRequest = GH.doesBranchHavePullRequest(branchName);
+			existingPullRequestPromise = GH.getPullRequestInfoForBranch(branchName);
 		}
 
 		let overwriteLocalBranch = !!options.force;
@@ -142,23 +142,18 @@ export const pickCommand = new Command()
 		log.debug(`Generated pull request body:\n${body}`);
 
 		// This promise is only set if it should be checked, undefined otherwise (await undefined = undefined)
-		const updatePR = await doesBranchHavePullRequest ?? false;
-		if (updatePR) {
+		const existingPR = await existingPullRequestPromise;
+		if (existingPR) {
 			log.info('PR exists, updating it!');
-			const currentPR = await GH.getPullRequestInfoForBranch(branchName);
-			if (!currentPR) {
-				log.warn('Unable to retrieve info for existing PR');
-			} else {
-				body = replacePRCLIPartOfBody(currentPR.body, body);
-			}
+			body = replacePRCLIPartOfBody(existingPR.body, body);
 		}
 
 		const settings = await confirmSettings(
 			{
 				push: options.push,
 				pr: options.pr,
-				updatePR,
-				draftPR: options.draft ?? false,
+				updatePR: !!existingPR,
+				draftPR: options.draft ?? existingPR?.isDraft ?? false,
 				pullRemote: options.pullRemote,
 				pushRemote: options.pushRemote,
 				overwriteLocalBranch,
@@ -173,7 +168,7 @@ export const pickCommand = new Command()
 		);
 
 		log.info('Go time!');
-		await runCherryPick(settings);
+		await runCherryPick(settings, { existingPR });
 
 		log.info(colors.bgGreen.brightWhite('✔ Done!'));
 	});

commands/pick/steps/git-pick.ts

@@ -1,7 +1,7 @@
 import { colors } from '@cliffy/ansi/colors';
 import type { Commit } from '../../../lib/git/commit.ts';
 import { Git } from '../../../lib/git/git.ts';
-import { GH } from '../../../lib/github/gh.ts';
+import { GH, type PullRequest } from '../../../lib/github/gh.ts';
 import { runQuietly } from '../../../lib/shell/shell.ts';
 import { sleep } from '../../../lib/sleep.ts';
 import * as log from '@std/log';
@@ -27,7 +27,10 @@ export type GitPickSettings = Readonly<{
 	body: string;
 }>;
 
-export async function runCherryPick(settings: GitPickSettings): Promise<void> {
+export async function runCherryPick(
+	settings: GitPickSettings,
+	context: { existingPR: PullRequest | null },
+): Promise<void> {
 	const cleanupSteps: Array<{ message: string; action: () => unknown | Promise<unknown> }> = [];
 
 	try {
@@ -83,6 +86,9 @@ export async function runCherryPick(settings: GitPickSettings): Promise<void> {
 
 				log.info(colors.green('▶️ Updating pull request'));
 				await GH.editPullRequest(prSettings);
+				if (!context.existingPR || context.existingPR.isDraft !== prSettings.draftPR) {
+					await GH.setPullRequestDraftStatus(prSettings.draftPR);
+				}
 			} else {
 				log.info(colors.green('▶️ Creating pull request'));
 				await GH.createPullRequest(prSettings);

lib/github/gh.ts

@@ -4,19 +4,19 @@ import { runAndCapture, runCommand } from '../shell/shell.ts';
 export const GH = {
 	createPullRequest,
 	editPullRequest,
-	doesBranchHavePullRequest,
+	setPullRequestDraftStatus,
 	listPullRequests,
 	getPullRequestInfoForCurrentBranch,
 	getPullRequestInfoForBranch,
 };
 
 interface BasePROptions {
 	baseBranch?: string;
-	draftPR?: boolean;
 }
 
 interface CreatePROptions {
 	web?: boolean;
+	draftPR?: boolean;
 }
 
 interface ManualTitleAndBody {
@@ -37,6 +37,20 @@ type PullRequestOptions =
 	& (ManualTitleAndBody | AutomaticTitleAndBody);
 type EditPullRequestOptions = BasePROptions & ManualTitleAndBody;
 
+export interface PullRequest {
+	title: string;
+	body: string;
+	number: number;
+	isDraft: boolean;
+	baseRefName: string;
+	headRefName: string;
+	commits: Array<{
+		messageHeadLine: string;
+		messageBody: string;
+		oid: string; // 464fc5a0d27f6053647cdb5939a936b91aaa91fc
+	}>;
+}
+
 async function getPullRequestInfoForCurrentBranch() {
 	const json = await runAndCapture(
 		'gh',
@@ -79,7 +93,6 @@ async function createPullRequest(options: PullRequestOptions) {
 async function editPullRequest(options: EditPullRequestOptions) {
 	const args: string[] = [];
 	options.baseBranch && args.push('--base', options.baseBranch);
-	options.draftPR && args.push('--draft');
 
 	await runCommand(
 		'gh',
@@ -95,19 +108,14 @@ async function editPullRequest(options: EditPullRequestOptions) {
 	);
 }
 
-async function doesBranchHavePullRequest(branch: string): Promise<boolean> {
-	try {
-		// gh pr view doesn't work for multi-remote use cases, gh pr list does
-		// This will give a false positive for prs from a different repository with the same branch name
-		const prs = await listPullRequests({ head: branch });
-		return prs.length > 0;
-	} catch {
-		return false;
-	}
+async function setPullRequestDraftStatus(draft: boolean) {
+	const args = draft ? ['--undo'] : [];
+	await runCommand('gh', 'pr', 'ready', ...args);
 }
 
 async function getPullRequestInfoForBranch(branch: string) {
 	// gh pr view doesn't work for multi-remote use cases, gh pr list does
+	// This will give a false positive for prs from a different repository with the same branch name
 	const prs = await listPullRequests({ head: branch });
 	if (prs.length < 1) {
 		return null;
@@ -135,20 +143,9 @@ async function listPullRequests(options: {
 		'list',
 		...args,
 		'--json',
-		'title,body,number,commits,headRefName,baseRefName',
+		'title,body,number,isDraft,commits,headRefName,baseRefName',
 	);
-	const prs: Array<{
-		title: string;
-		body: string;
-		number: number;
-		baseRefName: string;
-		headRefName: string;
-		commits: Array<{
-			messageHeadLine: string;
-			messageBody: string;
-			oid: string; // 464fc5a0d27f6053647cdb5939a936b91aaa91fc
-		}>;
-	}> = JSON.parse(json);
+	const prs: Array<PullRequest> = JSON.parse(json);
 
 	if (!Array.isArray(prs)) {
 		log.error('Invalid response from gh cli:', prs);