refactor: replace manual arg parsing in auth commands with clap#594
refactor: replace manual arg parsing in auth commands with clap#594
Conversation
- Add ScopeMode enum (Default, Readonly, Full, Custom) for type-safe scope selection - Build auth_command() with clap subcommands: login, setup, status, export, logout - Replace manual --help check and match dispatch in handle_auth_command with clap subcommand routing - Replace two-pass manual parsing in handle_login (services extraction) and resolve_scopes (scopes/readonly/full flags) with single clap parse - Add --unmasked flag to export subcommand via clap - Preserve run_login(&[]) public API for setup.rs compatibility - Update all 63 tests to use ScopeMode-based API
🦋 Changeset detectedLatest commit: b6fe1a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the command-line interface for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #594 +/- ##
==========================================
- Coverage 69.02% 69.00% -0.03%
==========================================
Files 42 42
Lines 19311 19371 +60
==========================================
+ Hits 13330 13367 +37
- Misses 5981 6004 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request is a great refactoring that replaces manual argument parsing with clap, significantly improving the robustness and maintainability of the gws auth subcommands. The introduction of the ScopeMode enum is a particularly nice touch for type safety.
I've identified a couple of areas for improvement related to code duplication and performance in the new clap integration. My comments provide suggestions to address these points by extracting duplicated logic into a helper function and using once_cell for efficient command initialization.
Overall, this is a high-quality change that modernizes the command-line handling. Well done.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great refactoring that replaces manual argument parsing with clap, improving robustness, maintainability, and user experience. The introduction of the ScopeMode enum is a good move for type safety. The changes are well-structured and the backward compatibility for setup commands is thoughtfully handled. I have one suggestion to make the command-line interface for gws auth login even more robust by making scope-related flags mutually exclusive.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great refactoring that replaces manual argument parsing with clap for the gws auth subcommands. This significantly improves the robustness, maintainability, and user-friendliness of the CLI by providing proper help messages, error handling for unknown flags, and type-safe argument parsing via the new ScopeMode enum. The changes are well-structured, and the test suite has been updated accordingly.
I have one suggestion to improve the robustness of custom scope parsing.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring that replaces manual argument parsing for auth commands with clap. The introduction of the ScopeMode enum and the clear separation of parsing logic greatly improve the code's robustness and maintainability. My review found one critical issue related to a potential runtime panic due to a fragile coupling between command definitions. Addressing this will make the implementation fully robust.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring that replaces manual argument parsing with clap for the gws auth subcommands. The changes significantly improve code clarity, maintainability, and user experience by providing proper help messages and argument validation. The introduction of the ScopeMode enum is a great improvement for type safety. I have one suggestion regarding duplicated code for clap error handling that could further enhance maintainability.
Summary
Replace manual argument parsing in all
gws authsubcommands with structuredclap::Commanddefinitions.Changes
ScopeModeenum — replaces raw arg passing toresolve_scopes, making scope selection type-safe (Default,Readonly,Full,Custom(Vec<String>))auth_command()— clap Command with subcommands:login,setup,status,export,logouthandle_auth_command— replaced hardcoded USAGE string + manual--helpcheck +match args[0]dispatch with clap subcommand routinghandle_login→handle_login_inner— eliminated two-pass manual parsing (services extraction loop + scopes/readonly/full flag scanning) with single clap parseresolve_scopes— now acceptsScopeModeinstead of raw&[String]argsexport --unmasked— handled via clap flag instead of manualargs[1] == "--unmasked"checkrun_login(&[])— public API preserved for setup.rs compatibilityBenefits
--helpsupport--helpexits cleanly (code 0), invalid flags return non-zeroTests
All 63 existing tests updated and passing.