Skip to content
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

feat(flags): script arguments come after '--' #3621

Merged
merged 14 commits into from
Jan 8, 2020
34 changes: 26 additions & 8 deletions cli/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ fn run_test_args_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {
fn run_parse(flags: &mut DenoFlags, matches: &clap::ArgMatches) {
flags.subcommand = DenoSubcommand::Run;
script_arg_parse(flags, matches);
args_parse(flags, matches);
run_test_args_parse(flags, matches);
}

Expand Down Expand Up @@ -997,6 +998,7 @@ fn run_subcommand<'a, 'b>() -> App<'a, 'b> {
run_test_args(SubCommand::with_name("run"))
.setting(AppSettings::TrailingVarArg)
.arg(script_arg())
.arg(args_arg())
.about("Run a program given a filename or url to the source code")
.long_about(
"Run a program given a filename or url to the source code.
Expand All @@ -1016,7 +1018,11 @@ With only permission to read from disk and listen to network

With only permission to read whitelist files from disk

deno run --allow-read=/etc https://deno.land/std/http/file_server.ts",
deno run --allow-read=/etc https://deno.land/std/http/file_server.ts

Any arguments that should be passed to the script should be prefixed by '--'

deno run -A https://deno.land/std/examples/cat.ts -- /etc/passwd",
)
}

Expand Down Expand Up @@ -1061,15 +1067,26 @@ _test.js and executes them.
}

fn script_arg<'a, 'b>() -> Arg<'a, 'b> {
Arg::with_name("script_arg")
.multiple(true)
.help("script args")
.value_name("SCRIPT_ARG")
Arg::with_name("script").help("script").value_name("SCRIPT")
}

fn script_arg_parse(flags: &mut DenoFlags, matches: &ArgMatches) {
if let Some(script_values) = matches.values_of("script_arg") {
for v in script_values {
if let Some(script_value) = matches.value_of("script") {
debug_assert!(flags.argv.len() == 1);
flags.argv.push(String::from(script_value));
}
}

fn args_arg<'a, 'b>() -> Arg<'a, 'b> {
Arg::with_name("script_args")
.raw(true)
.help("script args")
.value_name("SCRIPT_ARGS")
}

fn args_parse(flags: &mut DenoFlags, matches: &ArgMatches) {
if let Some(values) = matches.values_of("script_args") {
for v in values {
flags.argv.push(String::from(v));
}
}
Expand Down Expand Up @@ -1398,6 +1415,7 @@ mod tests {
"run",
"--allow-net",
"gist.ts",
"--",
"--title",
"X"
]);
Expand Down Expand Up @@ -1480,7 +1498,7 @@ mod tests {
r.unwrap(),
DenoFlags {
subcommand: DenoSubcommand::Run,
argv: svec!["deno", "script.ts", "--", "-D", "--allow-net"],
argv: svec!["deno", "script.ts", "-D", "--allow-net"],
allow_write: true,
..DenoFlags::default()
}
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ itest!(_027_redirect_typescript {
});

itest!(_028_args {
args: "run --reload 028_args.ts --arg1 val1 --arg2=val2 -- arg3 arg4",
args: "run --reload 028_args.ts -- --arg1 val1 --arg2=val2 -- arg3 arg4",
output: "028_args.ts.out",
});

Expand Down
1 change: 1 addition & 0 deletions cli/tests/std_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ mod tests {
.arg("-A")
// .arg("-Ldebug")
.arg("./testing/runner.ts")
.arg("--")
.arg("--exclude=testing/testdata")
.spawn()
.expect("failed to spawn script");
Expand Down
1 change: 1 addition & 0 deletions std/examples/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ test(async function catSmoke(): Promise<void> {
"run",
"--allow-read",
"examples/cat.ts",
"--",
"README.md"
],
stdout: "piped"
Expand Down
3 changes: 2 additions & 1 deletion std/fs/empty_dir_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,13 @@ test(async function emptyDirPermission(): Promise<void> {
args.push(
path.join(testdataDir, s.async ? "empty_dir.ts" : "empty_dir_sync.ts")
);
args.push("--");
args.push("testfolder");

const { stdout } = Deno.run({
stdout: "piped",
cwd: testdataDir,
args: args
args
});

const output = await Deno.readAll(stdout);
Expand Down
1 change: 1 addition & 0 deletions std/fs/exists_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ test(async function existsPermission(): Promise<void> {
}

args.push(path.join(testdataDir, s.async ? "exists.ts" : "exists_sync.ts"));
args.push("--");
args.push(s.file);

const { stdout } = Deno.run({
Expand Down
3 changes: 2 additions & 1 deletion std/http/file_server_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ async function startFileServer(): Promise<void> {
"--allow-read",
"--allow-net",
"http/file_server.ts",
"--",
".",
"--cors"
],
Expand Down Expand Up @@ -122,7 +123,7 @@ test(async function servePermissionDenied(): Promise<void> {

test(async function printHelp(): Promise<void> {
const helpProcess = Deno.run({
args: [Deno.execPath(), "run", "http/file_server.ts", "--help"],
args: [Deno.execPath(), "run", "http/file_server.ts", "--", "--help"],
stdout: "piped"
});
const r = new TextProtoReader(new BufReader(helpProcess.stdout!));
Expand Down
1 change: 1 addition & 0 deletions std/installer/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export async function install(
"run",
...grantedPermissions.map(getFlagFromPermission),
moduleUrl,
"--",
...scriptArgs
];

Expand Down
88 changes: 26 additions & 62 deletions std/installer/test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
const { run, stat, makeTempDir, remove, env, readAll } = Deno;
const { run, stat, makeTempDir, remove, env } = Deno;

import { test, runIfMain, TestFunction } from "../testing/mod.ts";
import { assert, assertEquals } from "../testing/asserts.ts";
Expand All @@ -20,6 +20,7 @@ async function startFileServer(): Promise<void> {
"--allow-read",
"--allow-net",
"http/file_server.ts",
"--",
".",
"--cors"
],
Expand Down Expand Up @@ -83,11 +84,11 @@ installerTest(async function installBasic(): Promise<void> {
/* eslint-disable max-len */
`% This executable is generated by Deno. Please don't modify it unless you know what it means. %
@IF EXIST "%~dp0\deno.exe" (
"%~dp0\deno.exe" "run" "http:https://localhost:4500/installer/testdata/echo.ts" %*
"%~dp0\deno.exe" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" %*
) ELSE (
@SETLOCAL
@SET PATHEXT=%PATHEXT:;.TS;=;%
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" %*
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" %*
)
`
/* eslint-enable max-len */
Expand All @@ -106,10 +107,10 @@ case \`uname\` in
esac

if [ -x "$basedir/deno" ]; then
"$basedir/deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "$@"
"$basedir/deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" "$@"
ret=$?
else
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "$@"
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" "$@"
ret=$?
fi
exit $ret
Expand Down Expand Up @@ -138,11 +139,11 @@ installerTest(async function installCustomDir(): Promise<void> {
/* eslint-disable max-len */
`% This executable is generated by Deno. Please don't modify it unless you know what it means. %
@IF EXIST "%~dp0\deno.exe" (
"%~dp0\deno.exe" "run" "http:https://localhost:4500/installer/testdata/echo.ts" %*
"%~dp0\deno.exe" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" %*
) ELSE (
@SETLOCAL
@SET PATHEXT=%PATHEXT:;.TS;=;%
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" %*
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" %*
)
`
/* eslint-enable max-len */
Expand All @@ -161,10 +162,10 @@ case \`uname\` in
esac

if [ -x "$basedir/deno" ]; then
"$basedir/deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "$@"
"$basedir/deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" "$@"
ret=$?
else
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "$@"
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" "$@"
ret=$?
fi
exit $ret
Expand Down Expand Up @@ -192,11 +193,11 @@ installerTest(async function installLocalModule(): Promise<void> {
/* eslint-disable max-len */
`% This executable is generated by Deno. Please don't modify it unless you know what it means. %
@IF EXIST "%~dp0\deno.exe" (
"%~dp0\deno.exe" "run" "${localModule}" %*
"%~dp0\deno.exe" "run" "${localModule}" "--" %*
) ELSE (
@SETLOCAL
@SET PATHEXT=%PATHEXT:;.TS;=;%
"deno" "run" "${localModule}" %*
"deno" "run" "${localModule}" "--" %*
)
`
/* eslint-enable max-len */
Expand All @@ -215,10 +216,10 @@ case \`uname\` in
esac

if [ -x "$basedir/deno" ]; then
"$basedir/deno" "run" "${localModule}" "$@"
"$basedir/deno" "run" "${localModule}" "--" "$@"
ret=$?
else
"deno" "run" "${localModule}" "$@"
"deno" "run" "${localModule}" "--" "$@"
ret=$?
fi
exit $ret
Expand All @@ -227,57 +228,17 @@ exit $ret
);
});

/* TODO(ry) Re-enable this test
installerTest(async function installWithFlags(): Promise<void> {
await install(
"echo_test",
"http:https://localhost:4500/installer/testdata/echo.ts",
["--allow-net", "--allow-read", "--foobar"]
);

const { HOME } = env();
const filePath = path.resolve(HOME, ".deno/bin/echo_test");

if (path.isWindows) {
assertEquals(
await fs.readFileStr(filePath + ".cmd"),
/* eslint-disable max-len */
`% This executable is generated by Deno. Please don't modify it unless you know what it means. %
@IF EXIST "%~dp0\deno.exe" (
"%~dp0\deno.exe" "run" "--allow-net" "--allow-read" "http:https://localhost:4500/installer/testdata/echo.ts" "--foobar" %*
) ELSE (
@SETLOCAL
@SET PATHEXT=%PATHEXT:;.TS;=;%
"deno" "run" "--allow-net" "--allow-read" "http:https://localhost:4500/installer/testdata/echo.ts" "--foobar" %*
)
`
/* eslint-enable max-len */
);
}

assertEquals(
await fs.readFileStr(filePath),
/* eslint-disable max-len */
`#!/bin/sh
# This executable is generated by Deno. Please don't modify it unless you know what it means.
basedir=$(dirname "$(echo "$0" | sed -e 's,\\\\,/,g')")

case \`uname\` in
*CYGWIN*) basedir=\`cygpath -w "$basedir"\`;;
esac

if [ -x "$basedir/deno" ]; then
"$basedir/deno" "run" "--allow-net" "--allow-read" "http:https://localhost:4500/installer/testdata/echo.ts" "--foobar" "$@"
ret=$?
else
"deno" "run" "--allow-net" "--allow-read" "http:https://localhost:4500/installer/testdata/echo.ts" "--foobar" "$@"
ret=$?
fi
exit $ret
`
/* eslint-enable max-len */
);
});
*/

/* TODO(ry) Re-enable this test
installerTest(async function installLocalModuleAndRun(): Promise<void> {
const tempDir = await makeTempDir();
const localModule = path.join(Deno.cwd(), "installer", "testdata", "echo.ts");
Expand Down Expand Up @@ -315,7 +276,9 @@ installerTest(async function installLocalModuleAndRun(): Promise<void> {

assert(!thrown, "It should not throw an error");
}, true); // set true to install module in your real $HOME dir.
*/

/* TODO(ry) Re-enable this test
installerTest(async function installAndMakesureItCanRun(): Promise<void> {
const tempDir = await makeTempDir();
await install(
Expand Down Expand Up @@ -357,7 +320,9 @@ installerTest(async function installAndMakesureItCanRun(): Promise<void> {

assert(!thrown, "It should not throw an error");
}, true); // set true to install module in your real $HOME dir.
*/

/* TODO(ry) Re-enable this test
installerTest(async function installAndMakesureArgsRight(): Promise<void> {
const tempDir = await makeTempDir();
await install(
Expand Down Expand Up @@ -385,9 +350,7 @@ installerTest(async function installAndMakesureArgsRight(): Promise<void> {

try {
const b = await readAll(ps.stdout);

const s = new TextDecoder("utf-8").decode(b);

const obj = JSON.parse(s);

assertEquals(obj[0], "arg1");
Expand All @@ -404,6 +367,7 @@ installerTest(async function installAndMakesureArgsRight(): Promise<void> {

assert(!thrown, "It should not throw an error");
}, true); // set true to install module in your real $HOME dir.
*/

installerTest(async function installWithoutHOMEVar(): Promise<void> {
const { HOME } = env();
Expand All @@ -427,11 +391,11 @@ installerTest(async function installWithoutHOMEVar(): Promise<void> {
/* eslint-disable max-len */
`% This executable is generated by Deno. Please don't modify it unless you know what it means. %
@IF EXIST "%~dp0\deno.exe" (
"%~dp0\deno.exe" "run" "http:https://localhost:4500/installer/testdata/echo.ts" %*
"%~dp0\deno.exe" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" %*
) ELSE (
@SETLOCAL
@SET PATHEXT=%PATHEXT:;.TS;=;%
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" %*
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" %*
)
`
/* eslint-enable max-len */
Expand All @@ -450,10 +414,10 @@ case \`uname\` in
esac

if [ -x "$basedir/deno" ]; then
"$basedir/deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "$@"
"$basedir/deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" "$@"
ret=$?
else
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "$@"
"deno" "run" "http:https://localhost:4500/installer/testdata/echo.ts" "--" "$@"
ret=$?
fi
exit $ret
Expand Down
1 change: 1 addition & 0 deletions std/manual.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ const p = Deno.run({
"run",
"--allow-read",
"https://deno.land/std/examples/cat.ts",
"--",
...fileNames
],
stdout: "piped",
Expand Down
Loading