Skip to content

Commit da8f10e

Browse files
author
Joevenner
committed
fix(auth): centralize base_dir resolution and fix arg parsing
Resolves code review feedback: - Extracted 'base_config_dir()' to a single location to prevent buggy fallback deduplication across 'auth_commands.rs' and 'credential_store.rs'. - Fixed 'main.rs' argument filtering logic which incorrectly skipped the values of the '--api-version' flag.
1 parent 8f474df commit da8f10e

3 files changed

Lines changed: 40 additions & 74 deletions

File tree

src/auth_commands.rs

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ const READONLY_SCOPES: &[&str] = &[
9191
"https://www.googleapis.com/auth/tasks.readonly",
9292
];
9393

94-
pub fn config_dir() -> PathBuf {
95-
let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") {
94+
pub fn base_config_dir() -> PathBuf {
95+
if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") {
9696
PathBuf::from(dir)
9797
} else {
9898
// Use ~/.config/gws on all platforms for a consistent, user-friendly path.
@@ -115,7 +115,11 @@ pub fn config_dir() -> PathBuf {
115115
primary
116116
}
117117
}
118-
};
118+
}
119+
}
120+
121+
pub fn config_dir() -> PathBuf {
122+
let base_dir = base_config_dir();
119123

120124
// Determine the active profile
121125
let mut profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE").ok().filter(|s| !s.is_empty());
@@ -199,21 +203,7 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> {
199203
let profile_name = &args[0];
200204

201205
// Read the base directory without applying the current active profile
202-
let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") {
203-
PathBuf::from(dir)
204-
} else {
205-
let primary = dirs::home_dir()
206-
.unwrap_or_else(|| PathBuf::from("."))
207-
.join(".config")
208-
.join("gws");
209-
if primary.exists() {
210-
primary
211-
} else {
212-
dirs::config_dir()
213-
.unwrap_or_else(|| PathBuf::from("."))
214-
.join("gws")
215-
}
216-
};
206+
let base_dir = base_config_dir();
217207

218208
if !base_dir.exists() {
219209
std::fs::create_dir_all(&base_dir).map_err(|e| {
@@ -1217,21 +1207,7 @@ async fn handle_status() -> Result<(), GwsError> {
12171207
);
12181208

12191209
// Determine the active profile
1220-
let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") {
1221-
PathBuf::from(dir)
1222-
} else {
1223-
let primary = dirs::home_dir()
1224-
.unwrap_or_else(|| PathBuf::from("."))
1225-
.join(".config")
1226-
.join("gws");
1227-
if primary.exists() {
1228-
primary
1229-
} else {
1230-
dirs::config_dir()
1231-
.unwrap_or_else(|| PathBuf::from("."))
1232-
.join("gws")
1233-
}
1234-
};
1210+
let base_dir = base_config_dir();
12351211

12361212
let profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE")
12371213
.ok()

src/credential_store.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
5252
.filter(|s| !s.is_empty())
5353
.or_else(|| {
5454
// Attempt to read from the base config dir's active_profile
55-
let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") {
56-
PathBuf::from(dir)
57-
} else {
58-
dirs::home_dir()
59-
.unwrap_or_else(|| PathBuf::from("."))
60-
.join(".config")
61-
.join("gws")
62-
};
55+
let base_dir = crate::auth_commands::base_config_dir();
6356
std::fs::read_to_string(base_dir.join("active_profile"))
6457
.ok()
6558
.map(|s| s.trim().to_string())

src/main.rs

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -64,42 +64,39 @@ async fn run() -> Result<(), GwsError> {
6464
));
6565
}
6666

67-
// Find the first non-flag arg (skip --api-version, --profile and their values)
68-
let mut first_arg: Option<String> = None;
6967
let mut filtered_args = vec![args[0].clone()];
70-
{
71-
let mut skip_next = false;
72-
for a in args.iter().skip(1) {
73-
if skip_next {
74-
skip_next = false;
75-
continue;
76-
}
77-
if a == "--api-version" {
78-
filtered_args.push(a.clone());
79-
skip_next = true;
80-
continue;
81-
}
82-
if a.starts_with("--api-version=") {
83-
filtered_args.push(a.clone());
84-
continue;
85-
}
86-
if a == "--profile" {
87-
skip_next = true;
88-
continue;
89-
}
90-
if a.starts_with("--profile=") {
91-
continue;
92-
}
93-
filtered_args.push(a.clone());
94-
95-
if first_arg.is_none()
96-
&& !a.starts_with("--")
97-
&& a.as_str() != "--help"
98-
&& a.as_str() != "--version"
99-
{
100-
first_arg = Some(a.clone());
68+
let mut first_arg: Option<String> = None;
69+
let mut i = 1;
70+
while i < args.len() {
71+
let a = &args[i];
72+
73+
if a == "--profile" {
74+
i += 2; // Skip --profile and its value
75+
continue;
76+
}
77+
if a.starts_with("--profile=") {
78+
i += 1;
79+
continue;
80+
}
81+
82+
filtered_args.push(a.clone());
83+
84+
if a == "--api-version" {
85+
if i + 1 < args.len() {
86+
filtered_args.push(args[i+1].clone());
10187
}
88+
i += 2; // Skip both the flag and the value
89+
continue;
90+
}
91+
if a.starts_with("--api-version=") {
92+
i += 1;
93+
continue;
94+
}
95+
96+
if first_arg.is_none() && !a.starts_with("--") && a != "--help" && a != "--version" {
97+
first_arg = Some(a.clone());
10298
}
99+
i += 1;
103100
}
104101

105102
// Extract profile early to set environment variable

0 commit comments

Comments
 (0)