Skip to content

Commit 93bdd18

Browse files
eddietejedaclaude
andcommitted
Address PR review: serde default, URL encoding, save warning, arg order
- `Database.attachments` gets `#[serde(default)]` so the server can omit the field without causing a hard parse failure - `try_resolve_database` percent-encodes the path segment via `urlencoding` so descriptions containing spaces don't crash the URL parse before the list fallback runs - `create` now warns (yellow) instead of silently discarding a config save failure, so the user knows the implicit "set as current" didn't take - `set(id_or_description, workspace_id)` → `set(workspace_id, id_or_description)` to match the argument order of every other public fn in databases.rs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent fc1e090 commit 93bdd18

4 files changed

Lines changed: 13 additions & 4 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ sysinfo = { version = "0.38.4", default-features = false, features = ["system"]
4040
self_update = { version = "0.42", default-features = false, features = ["rustls"] }
4141
lzma-rs = "0.3"
4242
tempfile = "3"
43+
urlencoding = "2.1.3"
4344

4445
[dev-dependencies]
4546
mockito = "1"

src/databases.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub struct Database {
2323
pub id: String,
2424
pub description: Option<String>,
2525
pub default_connection_id: String,
26+
#[serde(default)]
2627
attachments: Vec<DatabaseAttachment>,
2728
}
2829

@@ -82,7 +83,10 @@ fn fetch_database(api: &ApiClient, id: &str) -> Database {
8283

8384
pub fn try_resolve_database(api: &ApiClient, id_or_description: &str) -> Result<Database, String> {
8485
// Try a direct id lookup first — avoids the list round-trip for the common case.
85-
if let Some(db) = api.get_none_if_not_found(&format!("/databases/{id_or_description}")) {
86+
// Percent-encode the segment so descriptions containing spaces or other URL-unsafe
87+
// characters don't cause a URL parse error before the list fallback can run.
88+
let encoded = urlencoding::encode(id_or_description);
89+
if let Some(db) = api.get_none_if_not_found(&format!("/databases/{encoded}")) {
8690
return Ok(db);
8791
}
8892

@@ -436,7 +440,10 @@ pub fn create(
436440
}
437441
};
438442

439-
let _ = crate::config::save_current_database("default", workspace_id, &result.id);
443+
if let Err(e) = crate::config::save_current_database("default", workspace_id, &result.id) {
444+
use crossterm::style::Stylize;
445+
eprintln!("{}", format!("warning: database created but could not set as current: {e}").yellow());
446+
}
440447

441448
match format {
442449
"json" => println!("{}", serde_json::to_string_pretty(&result).unwrap()),
@@ -452,7 +459,7 @@ pub fn create(
452459
}
453460
}
454461

455-
pub fn set(id_or_description: &str, workspace_id: &str) {
462+
pub fn set(workspace_id: &str, id_or_description: &str) {
456463
use crossterm::style::Stylize;
457464
let api = ApiClient::new(Some(workspace_id));
458465
let db = resolve_database(&api, id_or_description);

src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ fn main() {
416416
&output,
417417
),
418418
Some(DatabasesCommands::Set { id_or_description }) => {
419-
databases::set(&id_or_description, &workspace_id)
419+
databases::set(&workspace_id, &id_or_description)
420420
}
421421
Some(DatabasesCommands::Delete { name_or_id }) => {
422422
databases::delete(&workspace_id, &name_or_id)

0 commit comments

Comments
 (0)