Skip to content

Add new api module and debug printing#25

Merged
pthurlow merged 3 commits intomainfrom
feat/debug
Mar 30, 2026
Merged

Add new api module and debug printing#25
pthurlow merged 3 commits intomainfrom
feat/debug

Conversation

@pthurlow
Copy link
Copy Markdown
Collaborator

No description provided.

claude[bot]
claude bot previously approved these changes Mar 28, 2026
Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review body: URL-encode query parameter values in query_string() at src/api.rs:244

Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 - src/api.rs:244 - Query parameter values are not URL-encoded. query_string() builds the query string with raw string interpolation. Cursor values from the API are typically base64-encoded and contain +, /, =. A + in a query string is decoded as a space by server-side parsers (application/x-www-form-urlencoded), silently corrupting the cursor. This breaks cursor-based pagination in tables::list. Action Required: URL-encode query parameter values in query_string() using urlencoding::encode.

_ => unreachable!(),
}
crate::query::print_result(&result, format);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: the old results::get showed [result-id: {id}] in the table footer and extracted error.message from error JSON. Both are now silently dropped. If this is intentional, no action needed — just worth confirming the UX change is expected.


let resp = match self.build_request(reqwest::Method::POST, &url)
.json(body)
.send()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: the doc comment says "exits on error" but this method only exits on connection failure — HTTP error responses are returned to the caller. Consider rewording to "exits on connection error, returns raw (status, body)".

@@ -111,44 +68,10 @@ pub fn list(format: &str) {
std::process::exit(1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: config is loaded here to get default_id, then ApiClient::new loads it again internally. Minor double-load — not worth restructuring now, just noting it.

@pthurlow pthurlow merged commit d203718 into main Mar 30, 2026
7 checks passed
@pthurlow pthurlow deleted the feat/debug branch March 30, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant