Skip to content

Commit c27bf2c

Browse files
Copilotalexr00
andcommitted
Address PR review feedback: extract helper function, add repo validation, revert title
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent d730c88 commit c27bf2c

File tree

3 files changed

+1551
-1567
lines changed

3 files changed

+1551
-1567
lines changed

package.nls.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@
197197
"command.pr.focusDescriptionInput.title": "Focus Pull Request Description Review Input",
198198
"command.pr.showDiffSinceLastReview.title": "Show Changes Since Last Review",
199199
"command.pr.showDiffAll.title": "Show All Changes",
200-
"command.pr.checkoutByNumber.title": "Checkout Pull Request by Number or URL",
200+
"command.pr.checkoutByNumber.title": "Checkout Pull Request by Number",
201201
"command.review.openFile.title": "Open File",
202202
"command.review.openLocalFile.title": "Open File",
203203
"command.review.suggestDiff.title": "Suggest Edit",

src/commands.ts

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,6 +1456,31 @@ ${contents}
14561456
}
14571457
}));
14581458

1459+
function validateAndParseInput(input: string, expectedOwner: string, expectedRepo: string): { isValid: boolean; prNumber?: number; errorMessage?: string } {
1460+
const prNumberMatcher = /^#?(\d*)$/;
1461+
const numberMatches = input.match(prNumberMatcher);
1462+
if (numberMatches && (numberMatches.length === 2) && !Number.isNaN(Number(numberMatches[1]))) {
1463+
const num = Number(numberMatches[1]);
1464+
if (num > 0) {
1465+
return { isValid: true, prNumber: num };
1466+
}
1467+
}
1468+
1469+
const urlMatches = input.match(ISSUE_OR_URL_EXPRESSION);
1470+
const parsed = parseIssueExpressionOutput(urlMatches);
1471+
if (parsed && parsed.issueNumber && parsed.issueNumber > 0) {
1472+
// Check if the repository owner and name match
1473+
if (parsed.owner && parsed.name) {
1474+
if (parsed.owner !== expectedOwner || parsed.name !== expectedRepo) {
1475+
return { isValid: false, errorMessage: vscode.l10n.t('Repository in URL does not match the selected repository') };
1476+
}
1477+
}
1478+
return { isValid: true, prNumber: parsed.issueNumber };
1479+
}
1480+
1481+
return { isValid: false, errorMessage: vscode.l10n.t('Value must be a pull request number or GitHub URL') };
1482+
}
1483+
14591484
context.subscriptions.push(
14601485
vscode.commands.registerCommand('pr.checkoutByNumber', async () => {
14611486

@@ -1473,52 +1498,24 @@ ${contents}
14731498
if (!githubRepo) {
14741499
return;
14751500
}
1476-
const prNumberMatcher = /^#?(\d*)$/;
14771501
const prNumber = await vscode.window.showInputBox({
14781502
ignoreFocusOut: true, prompt: vscode.l10n.t('Enter the pull request number or URL'),
14791503
validateInput: (input: string) => {
1480-
const prNumberMatcher = /^#?(\d*)$/;
1481-
const numberMatches = input.match(prNumberMatcher);
1482-
if (numberMatches && (numberMatches.length === 2) && !Number.isNaN(Number(numberMatches[1]))) {
1483-
const num = Number(numberMatches[1]);
1484-
if (num > 0) {
1485-
return undefined; // Valid number
1486-
}
1487-
}
1488-
1489-
const urlMatches = input.match(ISSUE_OR_URL_EXPRESSION);
1490-
const parsed = parseIssueExpressionOutput(urlMatches);
1491-
if (parsed && parsed.issueNumber && parsed.issueNumber > 0) {
1492-
return undefined; // Valid URL
1493-
}
1494-
1495-
return vscode.l10n.t('Value must be a pull request number or GitHub URL');
1504+
const result = validateAndParseInput(input, githubRepo.repo.remote.owner, githubRepo.repo.remote.repositoryName);
1505+
return result.isValid ? undefined : result.errorMessage;
14961506
}
14971507
});
14981508
if ((prNumber === undefined) || prNumber === '#') {
14991509
return;
15001510
}
15011511

15021512
// Extract PR number from input (either direct number or URL)
1503-
let extractedPrNumber: number;
1504-
const numberMatches = prNumber.match(prNumberMatcher);
1505-
if (numberMatches && (numberMatches.length === 2) && !Number.isNaN(Number(numberMatches[1]))) {
1506-
const num = Number(numberMatches[1]);
1507-
if (num > 0) {
1508-
extractedPrNumber = num;
1509-
} else {
1510-
return vscode.window.showErrorMessage(vscode.l10n.t('Invalid pull request number or URL'));
1511-
}
1512-
} else {
1513-
const urlMatches = prNumber.match(ISSUE_OR_URL_EXPRESSION);
1514-
const parsed = parseIssueExpressionOutput(urlMatches);
1515-
if (!parsed || !parsed.issueNumber || parsed.issueNumber <= 0) {
1516-
return vscode.window.showErrorMessage(vscode.l10n.t('Invalid pull request number or URL'));
1517-
}
1518-
extractedPrNumber = parsed.issueNumber;
1513+
const parseResult = validateAndParseInput(prNumber, githubRepo.repo.remote.owner, githubRepo.repo.remote.repositoryName);
1514+
if (!parseResult.isValid || !parseResult.prNumber) {
1515+
return vscode.window.showErrorMessage(parseResult.errorMessage || vscode.l10n.t('Invalid pull request number or URL'));
15191516
}
15201517

1521-
const prModel = await githubRepo.manager.fetchById(githubRepo.repo, extractedPrNumber);
1518+
const prModel = await githubRepo.manager.fetchById(githubRepo.repo, parseResult.prNumber);
15221519
if (prModel) {
15231520
return ReviewManager.getReviewManagerForFolderManager(reviewsManager.reviewManagers, githubRepo.manager)?.switch(prModel);
15241521
}

0 commit comments

Comments
 (0)