Skip to content

Commit 46dded7

Browse files
committed
fix: npm routing, discover cat redirect, proxy quoted args (#470, #315, #388)
#470: rtk npm now correctly routes npm subcommands (install, list, audit, etc.) without injecting "run". Previously, `rtk npm install` was executed as `npm run install`. #315: discover no longer counts `cat >`, `cat >>`, `cat |` as missed savings. These are write/pipe operations with no terminal output to compress. #388: rtk proxy now auto-splits a single quoted argument containing spaces. `rtk proxy 'head -50 file.php'` now works like `rtk proxy head -50 file.php`. Signed-off-by: Patrick szymkowiak <patrick.szymkowiak@innovtech.eu>
1 parent 18f4401 commit 46dded7

3 files changed

Lines changed: 249 additions & 43 deletions

File tree

src/discover/registry.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,23 @@ pub fn classify_command(cmd: &str) -> Classification {
7676
return Classification::Ignored;
7777
}
7878

79+
// Exclude cat/head/tail with redirect operators — these are writes, not reads (#315)
80+
if cmd_clean.starts_with("cat ")
81+
|| cmd_clean.starts_with("head ")
82+
|| cmd_clean.starts_with("tail ")
83+
{
84+
let after_cmd = cmd_clean.split_whitespace().nth(1).unwrap_or("");
85+
if after_cmd.starts_with('>') || after_cmd.starts_with('<') || after_cmd.starts_with('|') {
86+
return Classification::Unsupported {
87+
base_command: cmd_clean
88+
.split_whitespace()
89+
.next()
90+
.unwrap_or("cat")
91+
.to_string(),
92+
};
93+
}
94+
}
95+
7996
// Fast check with RegexSet — take the last (most specific) match
8097
let matches: Vec<usize> = REGEX_SET.matches(cmd_clean).into_iter().collect();
8198
if let Some(&idx) = matches.last() {
@@ -592,6 +609,23 @@ mod tests {
592609
);
593610
}
594611

612+
#[test]
613+
fn test_classify_cat_redirect_not_supported() {
614+
// cat > file and cat >> file are writes, not reads — should not be classified as supported
615+
match classify_command("cat > /tmp/output.txt") {
616+
Classification::Supported { .. } => {
617+
panic!("cat > should NOT be classified as Supported")
618+
}
619+
_ => {} // Unsupported or Ignored is fine
620+
}
621+
match classify_command("cat >> /tmp/output.txt") {
622+
Classification::Supported { .. } => {
623+
panic!("cat >> should NOT be classified as Supported")
624+
}
625+
_ => {}
626+
}
627+
}
628+
595629
#[test]
596630
fn test_classify_cd_ignored() {
597631
assert_eq!(classify_command("cd /tmp"), Classification::Ignored);

src/main.rs

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1857,17 +1857,34 @@ fn main() -> Result<()> {
18571857

18581858
let timer = tracking::TimedExecution::start();
18591859

1860-
let cmd_name = args[0].to_string_lossy();
1861-
let cmd_args: Vec<String> = args[1..]
1862-
.iter()
1863-
.map(|s| s.to_string_lossy().into_owned())
1864-
.collect();
1860+
// If a single quoted arg contains spaces, split it (#388).
1861+
// e.g. rtk proxy 'head -50 file.php' → cmd=head, args=["-50", "file.php"]
1862+
let (cmd_name, cmd_args): (String, Vec<String>) = if args.len() == 1 {
1863+
let full = args[0].to_string_lossy();
1864+
let parts: Vec<&str> = full.split_whitespace().collect();
1865+
if parts.len() > 1 {
1866+
(
1867+
parts[0].to_string(),
1868+
parts[1..].iter().map(|s| s.to_string()).collect(),
1869+
)
1870+
} else {
1871+
(full.into_owned(), vec![])
1872+
}
1873+
} else {
1874+
(
1875+
args[0].to_string_lossy().into_owned(),
1876+
args[1..]
1877+
.iter()
1878+
.map(|s| s.to_string_lossy().into_owned())
1879+
.collect(),
1880+
)
1881+
};
18651882

18661883
if cli.verbose > 0 {
18671884
eprintln!("Proxy mode: {} {}", cmd_name, cmd_args.join(" "));
18681885
}
18691886

1870-
let mut child = Command::new(cmd_name.as_ref())
1887+
let mut child = Command::new(&cmd_name)
18711888
.args(&cmd_args)
18721889
.stdout(Stdio::piped())
18731890
.stderr(Stdio::piped())

src/npm_cmd.rs

Lines changed: 192 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,92 @@ pub fn run(args: &[String], verbose: u8, skip_env: bool) -> Result<()> {
66
let timer = tracking::TimedExecution::start();
77

88
let mut cmd = Command::new("npm");
9-
cmd.arg("run");
109

11-
// Strip leading "run" to avoid doubling (rtk npm run build → npm run build, not npm run run build)
12-
let effective_args = if args.first().map(|s| s.as_str()) == Some("run") {
10+
// Determine if this is "npm run <script>" or another npm subcommand (install, list, etc.)
11+
// Only inject "run" when args look like a script name, not a known npm subcommand.
12+
let npm_subcommands = [
13+
"install",
14+
"i",
15+
"ci",
16+
"uninstall",
17+
"remove",
18+
"rm",
19+
"update",
20+
"up",
21+
"list",
22+
"ls",
23+
"outdated",
24+
"init",
25+
"create",
26+
"publish",
27+
"pack",
28+
"link",
29+
"audit",
30+
"fund",
31+
"exec",
32+
"explain",
33+
"why",
34+
"search",
35+
"view",
36+
"info",
37+
"show",
38+
"config",
39+
"set",
40+
"get",
41+
"cache",
42+
"prune",
43+
"dedupe",
44+
"doctor",
45+
"help",
46+
"version",
47+
"prefix",
48+
"root",
49+
"bin",
50+
"bugs",
51+
"docs",
52+
"home",
53+
"repo",
54+
"ping",
55+
"whoami",
56+
"token",
57+
"profile",
58+
"team",
59+
"access",
60+
"owner",
61+
"deprecate",
62+
"dist-tag",
63+
"star",
64+
"stars",
65+
"login",
66+
"logout",
67+
"adduser",
68+
"unpublish",
69+
"pkg",
70+
"diff",
71+
"rebuild",
72+
"test",
73+
"t",
74+
"start",
75+
"stop",
76+
"restart",
77+
];
78+
79+
let first_arg = args.first().map(|s| s.as_str());
80+
let is_run_explicit = first_arg == Some("run");
81+
let is_npm_subcommand = first_arg
82+
.map(|a| npm_subcommands.contains(&a) || a.starts_with('-'))
83+
.unwrap_or(false);
84+
85+
let effective_args = if is_run_explicit {
86+
// "rtk npm run build" → "npm run build"
87+
cmd.arg("run");
1388
&args[1..]
89+
} else if is_npm_subcommand {
90+
// "rtk npm install express" → "npm install express"
91+
args
1492
} else {
93+
// "rtk npm build" → "npm run build" (assume script name)
94+
cmd.arg("run");
1595
args
1696
};
1797

@@ -24,10 +104,10 @@ pub fn run(args: &[String], verbose: u8, skip_env: bool) -> Result<()> {
24104
}
25105

26106
if verbose > 0 {
27-
eprintln!("Running: npm run {}", effective_args.join(" "));
107+
eprintln!("Running: npm {}", args.join(" "));
28108
}
29109

30-
let output = cmd.output().context("Failed to run npm run")?;
110+
let output = cmd.output().context("Failed to run npm")?;
31111
let stdout = String::from_utf8_lossy(&output.stdout);
32112
let stderr = String::from_utf8_lossy(&output.stderr);
33113
let raw = format!("{}\n{}", stdout, stderr);
@@ -36,8 +116,8 @@ pub fn run(args: &[String], verbose: u8, skip_env: bool) -> Result<()> {
36116
println!("{}", filtered);
37117

38118
timer.track(
39-
&format!("npm run {}", effective_args.join(" ")),
40-
&format!("rtk npm run {}", effective_args.join(" ")),
119+
&format!("npm {}", args.join(" ")),
120+
&format!("rtk npm {}", args.join(" ")),
41121
&raw,
42122
&filtered,
43123
);
@@ -108,36 +188,111 @@ npm notice
108188
}
109189

110190
#[test]
111-
fn test_strip_leading_run_from_args() {
112-
// When user runs `rtk npm run build`, args = ["run", "build"]
113-
// The "run" should be stripped since cmd.arg("run") already adds it
114-
let args: Vec<String> = vec!["run".into(), "build".into()];
115-
let effective_args = if args.first().map(|s| s.as_str()) == Some("run") {
116-
&args[1..]
117-
} else {
118-
&args[..]
119-
};
120-
assert_eq!(effective_args, &["build"]);
121-
122-
// When user runs `rtk npm build`, args = ["build"]
123-
// No stripping needed
124-
let args2: Vec<String> = vec!["build".into()];
125-
let effective_args2 = if args2.first().map(|s| s.as_str()) == Some("run") {
126-
&args2[1..]
127-
} else {
128-
&args2[..]
129-
};
130-
assert_eq!(effective_args2, &["build"]);
131-
132-
// When user runs `rtk npm run`, args = ["run"]
133-
// Strip "run" → empty args (npm run with no script)
134-
let args3: Vec<String> = vec!["run".into()];
135-
let effective_args3 = if args3.first().map(|s| s.as_str()) == Some("run") {
136-
&args3[1..]
137-
} else {
138-
&args3[..]
139-
};
140-
assert!(effective_args3.is_empty());
191+
fn test_npm_subcommand_detection() {
192+
// Known npm subcommands should NOT get "run" injected
193+
let npm_subcommands = [
194+
"install",
195+
"i",
196+
"ci",
197+
"uninstall",
198+
"list",
199+
"ls",
200+
"outdated",
201+
"test",
202+
"t",
203+
"start",
204+
"audit",
205+
"init",
206+
];
207+
for subcmd in &npm_subcommands {
208+
assert!(
209+
[
210+
"install",
211+
"i",
212+
"ci",
213+
"uninstall",
214+
"remove",
215+
"rm",
216+
"update",
217+
"up",
218+
"list",
219+
"ls",
220+
"outdated",
221+
"init",
222+
"create",
223+
"publish",
224+
"pack",
225+
"link",
226+
"audit",
227+
"fund",
228+
"exec",
229+
"explain",
230+
"why",
231+
"search",
232+
"view",
233+
"info",
234+
"show",
235+
"config",
236+
"set",
237+
"get",
238+
"cache",
239+
"prune",
240+
"dedupe",
241+
"doctor",
242+
"help",
243+
"version",
244+
"prefix",
245+
"root",
246+
"bin",
247+
"bugs",
248+
"docs",
249+
"home",
250+
"repo",
251+
"ping",
252+
"whoami",
253+
"token",
254+
"profile",
255+
"team",
256+
"access",
257+
"owner",
258+
"deprecate",
259+
"dist-tag",
260+
"star",
261+
"stars",
262+
"login",
263+
"logout",
264+
"adduser",
265+
"unpublish",
266+
"pkg",
267+
"diff",
268+
"rebuild",
269+
"test",
270+
"t",
271+
"start",
272+
"stop",
273+
"restart"
274+
]
275+
.contains(subcmd),
276+
"{} should be a known npm subcommand",
277+
subcmd
278+
);
279+
}
280+
281+
// "run" is explicit → strip it
282+
let args_run: Vec<String> = vec!["run".into(), "build".into()];
283+
assert_eq!(args_run.first().map(|s| s.as_str()), Some("run"));
284+
285+
// "build" is NOT a known subcommand → inject "run"
286+
assert!(
287+
!["install", "i", "ci", "list", "ls", "test", "t"].contains(&"build"),
288+
"build should not be a known npm subcommand"
289+
);
290+
291+
// "install" IS a known subcommand → no "run" injection
292+
assert!(
293+
["install", "i", "ci", "list", "ls", "test", "t"].contains(&"install"),
294+
"install should be a known npm subcommand"
295+
);
141296
}
142297

143298
#[test]

0 commit comments

Comments
 (0)