-
Notifications
You must be signed in to change notification settings - Fork 1.1k
docs: add helper command guidelines and anti-patterns #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,66 +1,95 @@ | ||
| # Helper Modules | ||
| # Helper Commands (`+verb`) — Guidelines | ||
|
|
||
| This directory contains "Helper" implementations that provide high-value, simplified commands for complex Google Workspace API operations. | ||
| ## Design Principle | ||
|
|
||
| ## Philosophy | ||
| The core design of `gws` is **schema-driven**: commands are dynamically generated from Google Discovery Documents at runtime. This avoids maintaining a hardcoded, unbounded argument surface. **Helpers must complement this design, not duplicate it.** | ||
|
|
||
| The goal of the `gws` CLI is to provide raw access to the Google Workspace APIs. However, some operations are common but complex to execute via raw API calls (e.g., sending an email, appending a row to a sheet). | ||
| ## When a Helper is Justified | ||
|
|
||
| **Helper commands should only be added if they offer "High Usefulness":** | ||
| A `+helper` command should exist only when it provides value that Discovery-based commands **cannot**: | ||
|
|
||
| * **Complex Abstraction:** Does it abstract away significant complexity (e.g., MIME encoding, complex JSON structures, multiple API calls)? | ||
| * **Format Conversion:** Does it handle data format conversions that are tedious for the user? | ||
| * **Not Just an Alias:** Avoid adding helpers that simply alias a single, straightforward API call. | ||
| | Justification | Example | Why Discovery Can't Do It | | ||
| |---|---|---| | ||
| | **Multi-step orchestration** | `+subscribe` | Creates Pub/Sub topic → subscription → Workspace Events subscription (3 APIs) | | ||
| | **Format translation** | `+write` | Transforms Markdown → Docs `batchUpdate` JSON | | ||
| | **Multi-API composition** | `+triage` | Lists messages then fetches N metadata payloads concurrently | | ||
| | **Complex body construction** | `+send`, `+reply` | Builds RFC 2822 MIME from simple flags | | ||
| | **Multipart upload** | `+upload` | Handles resumable upload protocol with progress | | ||
| | **Workflow recipes** | `+standup-report` | Chains calls across multiple services | | ||
|
|
||
| **Litmus test:** Can the user achieve the same result with `gws <service> <resource> <method> --params '{...}'`? If yes, don't add a helper. | ||
|
|
||
| ## Anti-Patterns | ||
|
|
||
| ### ❌ Anti-pattern 1: Single API Call Wrapper | ||
|
|
||
| If a helper wraps one API call that Discovery already exposes, reject it. | ||
|
|
||
| **Real example:** `+revisions` (PR #563) wrapped `gws drive files-revisions list` — same single API call, zero added value. | ||
|
|
||
| ### ❌ Anti-pattern 2: Unbounded Flag Accumulation | ||
|
|
||
| Adding flags to expose data that is already in the API response creates unbounded surface area. | ||
|
|
||
| **Real example:** `--thread-id`, `--delivered-to`, `--sent-last` on `+triage` (PR #597) — all three values are already present in the Gmail API response. Agents and users should extract them with `--format` or `jq`, not new flags. | ||
|
|
||
| **Why this is harmful:** Every API response contains dozens of fields. If we add a flag for each one, helpers become unbounded maintenance burdens — the exact problem Discovery-driven design solves. | ||
|
|
||
| ### ❌ Anti-pattern 3: Duplicating Discovery Parameters | ||
|
|
||
| Don't re-expose Discovery-defined parameters (e.g., `pageSize`, `fields`, `orderBy`) as custom helper flags. Use `--params` passthrough instead. | ||
|
|
||
| ## Flag Design Rules | ||
|
|
||
| Helper flags must control **orchestration logic**, not API parameters or output fields. | ||
|
|
||
| ### ✅ Good Flags (control orchestration) | ||
|
|
||
| | Flag | Helper | Why It's Good | | ||
| |---|---|---| | ||
| | `--spreadsheet`, `--range` | `+read` | Identifies which resource to operate on | | ||
| | `--to`, `--subject`, `--body` | `+send` | Inputs to MIME construction (format translation) | | ||
| | `--dry-run` | `+subscribe` | Controls whether API calls are actually made | | ||
| | `--subscription` | `+subscribe` | Switches between "create new" vs. "use existing" orchestration path | | ||
| | `--target`, `--project` | `+subscribe` | Required for multi-service resource creation | | ||
|
|
||
| ### ❌ Bad Flags (expose API response data) | ||
|
|
||
| | Flag | Why It's Bad | Alternative | | ||
| |---|---|---| | ||
| | `--thread-id` | Already in API response | `jq '.threadId'` | | ||
| | `--delivered-to` | Already in response headers | `jq '.payload.headers[] | ...'` | | ||
| | `--include-labels` | Output field filtering | `--format` or `jq` | | ||
|
|
||
| ### Decision Checklist for New Flags | ||
|
|
||
| 1. Does this flag control **what API call to make** or **how to orchestrate** multiple calls? → ✅ Add it | ||
| 2. Does this flag control **what data appears in output**? → ❌ Use `--format`/`jq` | ||
| 3. Does this flag duplicate a Discovery parameter? → ❌ Use `--params` | ||
| 4. Could the user achieve this with existing flags + post-processing? → ❌ Don't add it | ||
|
|
||
| ## Architecture | ||
|
|
||
jpoehnelt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Helpers are implemented using the `Helper` trait defined in `mod.rs`. | ||
|
|
||
| ```rust | ||
| pub trait Helper: Send + Sync { | ||
| fn inject_commands( | ||
| &self, | ||
| cmd: Command, | ||
| doc: &crate::discovery::RestDescription, | ||
| ) -> Command; | ||
|
|
||
| fn handle<'a>( | ||
| &'a self, | ||
| doc: &'a crate::discovery::RestDescription, | ||
| matches: &'a ArgMatches | ||
| ) -> Pin<Box<dyn Future<Output = Result<bool, GwsError>> + Send + 'a>>; | ||
| } | ||
| ``` | ||
|
|
||
| * **`inject_commands`**: Adds subcommands to the main service command. All helper commands are always shown regardless of authentication state. | ||
| * **`handle`**: implementation of the command logic. Returns `Ok(true)` if the command was handled, or `Ok(false)` to let the default raw resource handler attempt to handle it. | ||
|
|
||
| ### Catalogue | ||
|
|
||
| | Service | Command | Usage | Description | Equivalent Raw Command (Example) | | ||
| | :--- | :--- | :--- | :--- | :--- | | ||
| | **Gmail** | `+send` | `gws gmail +send ...` | Sends an email. | `gws gmail users messages send ...` | | ||
| | **Sheets** | `+append` | `gws sheets +append ...` | Appends a row. | `gws sheets spreadsheets values append ...` | | ||
| | **Sheets** | `+read` | `gws sheets +read ...` | Reads values. | `gws sheets spreadsheets values get ...` | | ||
| | **Docs** | `+write` | `gws docs +write ...` | Appends text. | `gws docs documents batchUpdate ...` | | ||
| | **Chat** | `+send` | `gws chat +send ...` | Sends message. | `gws chat spaces messages create ...` | | ||
| | **Drive** | `+upload` | `gws drive +upload ...` | Uploads file. | `gws drive files create --upload ...` | | ||
| | **Calendar** | `+insert` | `gws calendar +insert ...` | Creates event. | `gws calendar events insert ...` | | ||
| | **Script** | `+push` | `gws script +push --script <ID>` | Pushes files. | `gws script projects updateContent ...` | | ||
| | **Events** | `+subscribe` | `gws events +subscribe ...` | Subscribe & stream events. | Pub/Sub REST + Workspace Events API | | ||
| | **Events** | `+renew` | `gws events +renew ...` | Renew subscriptions. | `gws events subscriptions reactivate ...` | | ||
|
|
||
| ### Development | ||
|
|
||
| To add a new helper: | ||
| 1. Create `src/helpers/<service>.rs`. | ||
| 2. Implement the `Helper` trait. | ||
| 3. Register it in `src/helpers/mod.rs`. | ||
| 4. **Prefix** the command with `+` (e.g., `+create`). | ||
|
|
||
| ## Current Helpers | ||
|
|
||
| * **Gmail**: Sending emails (abstracts RFC 2822 encoding). | ||
| * **Sheets**: Appending rows (abstracts `ValueRange` JSON construction). | ||
| * **Docs**: Appending text (abstracts `batchUpdate` requests). | ||
| * **Chat**: Sending messages to spaces. | ||
| - **`inject_commands`**: Adds subcommands to the main service command. All helper commands are always shown regardless of authentication state. | ||
| - **`handle`**: Implementation of the command logic. Returns `Ok(true)` if the command was handled, or `Ok(false)` to let the default raw resource handler attempt to handle it. | ||
|
|
||
| ## Adding a New Helper — Checklist | ||
|
|
||
| 1. **Passes the litmus test** — cannot be done with a single Discovery command | ||
| 2. **Flags are bounded** — only flags controlling orchestration, not API params/output | ||
| 3. **Uses shared infrastructure:** | ||
| - `crate::client::build_client()` for HTTP | ||
| - `crate::validate::validate_resource_name()` for user-supplied resource IDs | ||
jpoehnelt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - `crate::validate::encode_path_segment()` for URL path segments | ||
| - `crate::output::sanitize_for_terminal()` for error messages | ||
| 4. **Has tests** — at minimum: command registration, required args, happy path | ||
| 5. **Supports `--dry-run`** where the helper creates or mutates resources | ||
|
|
||
| ### Development Steps | ||
|
|
||
| 1. Create `src/helpers/<service>.rs` | ||
| 2. Implement the `Helper` trait | ||
| 3. Register it in `src/helpers/mod.rs` | ||
| 4. **Prefix** the command with `+` (e.g., `+create`) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.