Skip to content

Commit cb25a7e

Browse files
Refactor: Standardize error reporting for argument failures
- Removes `parser.print_help()` calls from all error exit paths related to missing or invalid script arguments. - Appends a consistent ' (use --help for more details)' note to these error messages. - This provides more concise error output while still guiding the user to help if needed. - Affects errors for token, URL, owner/repo specification, and PR number determination.
1 parent 6b069dd commit cb25a7e

File tree

1 file changed

+48
-45
lines changed

1 file changed

+48
-45
lines changed

scripts/gha/get_pr_review_comments_standalone.py

Lines changed: 48 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ def parse_repo_url(url_string):
269269
)
270270

271271
args = parser.parse_args()
272+
error_suffix = " (use --help for more details)"
272273

273274
if not args.token:
274-
sys.stderr.write("Error: GitHub token not provided. Set GITHUB_TOKEN or use --token.\n")
275+
sys.stderr.write(f"Error: GitHub token not provided. Set GITHUB_TOKEN or use --token.{error_suffix}\n")
275276
sys.exit(1)
276277

277278
final_owner = None
@@ -283,47 +284,57 @@ def parse_repo_url(url_string):
283284
# 3. Else (neither --url nor --owner/--repo pair explicitly set), use auto-detected values (which are defaults for args.owner/args.repo).
284285

285286
if args.url:
286-
if args.owner != determined_owner or args.repo != determined_repo: # Check if owner/repo were explicitly changed from defaults
287-
sys.stderr.write("Error: Cannot use --owner/--repo when --url is specified.\n")
288-
parser.print_help()
287+
# URL is provided, it takes precedence.
288+
# Error if --owner or --repo were also explicitly set by the user (i.e., different from their defaults)
289+
owner_explicitly_set = args.owner is not None and args.owner != determined_owner
290+
repo_explicitly_set = args.repo is not None and args.repo != determined_repo
291+
if owner_explicitly_set or repo_explicitly_set:
292+
sys.stderr.write(f"Error: Cannot use --owner or --repo when --url is specified.{error_suffix}\n")
289293
sys.exit(1)
294+
290295
parsed_owner, parsed_repo = parse_repo_url(args.url)
291296
if parsed_owner and parsed_repo:
292297
final_owner = parsed_owner
293298
final_repo = parsed_repo
294299
sys.stderr.write(f"Using repository from --url: {final_owner}/{final_repo}\n")
295300
else:
296-
sys.stderr.write(f"Error: Invalid URL format provided: {args.url}. Expected https://github.com/owner/repo or [email protected]:owner/repo.git\n")
297-
parser.print_help()
301+
sys.stderr.write(f"Error: Invalid URL format: {args.url}. Expected https://github.com/owner/repo or [email protected]:owner/repo.git{error_suffix}\n")
298302
sys.exit(1)
299-
elif (args.owner != determined_owner or args.repo != determined_repo) or (not determined_owner and args.owner and args.repo):
300-
# This condition means:
301-
# 1. User explicitly set --owner or --repo to something different than auto-detected OR
302-
# 2. Auto-detection failed (determined_owner is None) AND user provided both owner and repo.
303-
if not args.owner or not args.repo:
304-
sys.stderr.write("Error: Both --owner and --repo must be specified if one is provided (and --url is not used).\n")
305-
parser.print_help()
303+
else:
304+
# URL is not provided, try to use owner/repo.
305+
# These could be from explicit user input or from auto-detection (via defaults).
306+
is_owner_from_user = args.owner is not None and args.owner != determined_owner
307+
is_repo_from_user = args.repo is not None and args.repo != determined_repo
308+
is_owner_from_default = args.owner is not None and args.owner == determined_owner and determined_owner is not None
309+
is_repo_from_default = args.repo is not None and args.repo == determined_repo and determined_repo is not None
310+
311+
if (is_owner_from_user or is_repo_from_user): # User explicitly set at least one
312+
if args.owner and args.repo: # Both were set (either explicitly or one explicit, one default that existed)
313+
final_owner = args.owner
314+
final_repo = args.repo
315+
sys.stderr.write(f"Using repository from --owner/--repo args: {final_owner}/{final_repo}\n")
316+
else: # User set one but not the other, and the other wasn't available from a default
317+
sys.stderr.write(f"Error: Both --owner and --repo must be specified if one is provided explicitly (and --url is not used).{error_suffix}\n")
318+
sys.exit(1)
319+
elif args.owner and args.repo: # Both are from successful auto-detection (already printed "Determined repository...")
320+
final_owner = args.owner
321+
final_repo = args.repo
322+
elif args.owner or args.repo: # Only one was available (e.g. auto-detect only found one, or bad default state)
323+
sys.stderr.write(f"Error: Both --owner and --repo are required if not using --url, and auto-detection was incomplete.{error_suffix}\n")
306324
sys.exit(1)
307-
final_owner = args.owner
308-
final_repo = args.repo
309-
sys.stderr.write(f"Using repository from --owner/--repo args: {final_owner}/{final_repo}\n")
310-
elif args.owner and args.repo: # Using auto-detected values which are now in args.owner and args.repo
311-
final_owner = args.owner
312-
final_repo = args.repo
313-
# The "Determined repository..." message was already printed if successful.
314-
else: # Handles cases like only one of owner/repo being set after defaults, or auto-detection failed and nothing/partial was given.
315-
if (args.owner and not args.repo) or (not args.owner and args.repo):
316-
sys.stderr.write("Error: Both --owner and --repo must be specified if one is provided (and --url is not used).\n")
317-
parser.print_help()
318-
sys.exit(1)
319-
# If it reaches here and final_owner/repo are still None, it means auto-detection failed and user didn't provide valid pair.
325+
# If none of the above, final_owner/repo remain None, will be caught by the check below.
320326

321327
if not final_owner or not final_repo:
322-
sys.stderr.write("Error: Could not determine repository. Please specify --url, OR both --owner and --repo, OR ensure git remote 'origin' is configured correctly.\n")
323-
sys.exit(1) # No print_help() here, as per request
328+
sys.stderr.write(f"Error: Could not determine repository. Please specify --url, OR both --owner and --repo, OR ensure git remote 'origin' is configured correctly.{error_suffix}\n")
329+
sys.exit(1)
330+
331+
332+
if not final_owner or not final_repo:
333+
sys.stderr.write(f"Error: Could not determine repository. Please specify --url, OR both --owner and --repo, OR ensure git remote 'origin' is configured correctly.{error_suffix}\n")
334+
sys.exit(1)
324335

325336
if not set_repo_url_standalone(final_owner, final_repo): # Sets global OWNER and REPO
326-
sys.stderr.write(f"Error: Could not set repository to {final_owner}/{final_repo}. Ensure owner/repo are correct.\n")
337+
sys.stderr.write(f"Error: Could not set repository to {final_owner}/{final_repo}. Ensure owner/repo are correct.{error_suffix}\n")
327338
sys.exit(1)
328339

329340
pull_request_number = args.pull_number
@@ -337,27 +348,19 @@ def parse_repo_url(url_string):
337348
if pull_request_number:
338349
sys.stderr.write(f"Found PR #{pull_request_number} for branch {current_branch_for_pr_check}.\n")
339350
else:
340-
sys.stderr.write(f"No open PR found for branch {current_branch_for_pr_check} in {OWNER}/{REPO}.\n")
351+
sys.stderr.write(f"No open PR found for branch {current_branch_for_pr_check} in {OWNER}/{REPO}.\n") # This is informational, error below
341352
else:
342-
# Error: Could not determine current git branch (for PR detection).
343-
sys.stderr.write("Error: Could not determine current git branch. Cannot find PR automatically.\n")
344-
sys.exit(1) # No print_help() here, as per request
353+
sys.stderr.write(f"Error: Could not determine current git branch. Cannot find PR automatically.{error_suffix}\n")
354+
sys.exit(1)
345355

346356
if not pull_request_number:
347-
# This is reached if:
348-
# - --pull_number was not specified AND
349-
# - current branch was determined BUT no PR was found for it.
350-
# (The cases where branch itself couldn't be determined now exit above without print_help)
351-
error_message = "Error: Pull request number not specified and no open PR found for the current branch."
352-
if args.pull_number: # Should not happen if logic is correct, but as a safeguard
353-
error_message = f"Error: Pull request number {args.pull_number} is invalid or not found."
354-
elif not current_branch_for_pr_check: # Should be caught by exit above
357+
error_message = "Error: Pull request number is required."
358+
if not args.pull_number and current_branch_for_pr_check: # Auto-detect branch ok, but no PR found
359+
error_message = f"Error: Pull request number not specified and no open PR found for branch {current_branch_for_pr_check}."
360+
elif not args.pull_number and not current_branch_for_pr_check: # Should have been caught above
355361
error_message = "Error: Pull request number not specified and could not determine current git branch."
356362

357-
sys.stderr.write(error_message + "\n")
358-
# Keeping print_help() for this more general failure case unless explicitly asked to remove.
359-
# This specific error (no PR found for a valid branch) is different from "can't find branch".
360-
parser.print_help()
363+
sys.stderr.write(f"{error_message}{error_suffix}\n")
361364
sys.exit(1)
362365

363366
sys.stderr.write(f"Fetching comments for PR #{pull_request_number} from {OWNER}/{REPO}...\n")

0 commit comments

Comments
 (0)