Revert "feat(flags): script arguments come after '--'" (denoland#3681)

Due to complaints about ergonomics and because it breaks shebang on
linux.

This reverts commit 2d5457d.

BREAKING CHANGE

cli/flags.rs

@@ -485,7 +485,6 @@ 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);
 }
 
@@ -925,7 +924,6 @@ 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.
@@ -945,11 +943,7 @@ 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
-
-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",
+ deno run --allow-read=/etc https://deno.land/std/http/file_server.ts",
  )
 }
 
@@ -994,26 +988,15 @@ _test.js and executes them.
 }
 
 fn script_arg<'a, 'b>() -> Arg<'a, 'b> {
- Arg::with_name("script").help("script").value_name("SCRIPT")
-}
-
-fn script_arg_parse(flags: &mut DenoFlags, matches: &ArgMatches) {
- 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)
+ Arg::with_name("script_arg")
+ .multiple(true)
  .help("script args")
- .value_name("SCRIPT_ARGS")
+ .value_name("SCRIPT_ARG")
 }
 
-fn args_parse(flags: &mut DenoFlags, matches: &ArgMatches) {
- if let Some(values) = matches.values_of("script_args") {
- for v in values {
+fn script_arg_parse(flags: &mut DenoFlags, matches: &ArgMatches) {
+ if let Some(script_values) = matches.values_of("script_arg") {
+ for v in script_values {
  flags.argv.push(String::from(v));
  }
  }
@@ -1341,7 +1324,6 @@ mod tests {
  "run",
  "--allow-net",
  "gist.ts",
- "--",
  "--title",
  "X"
  ]);
@@ -1424,7 +1406,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()
  }

cli/tests/integration_tests.rs

@@ -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",
 });
 

cli/tests/std_tests.rs

@@ -23,7 +23,6 @@ mod tests {
  .arg("-A")
  // .arg("-Ldebug")
  .arg("./testing/runner.ts")
- .arg("--")
  .arg("--exclude=testing/testdata")
  .spawn()
  .expect("failed to spawn script");

std/examples/test.ts

@@ -20,7 +20,6 @@ test(async function catSmoke(): Promise<void> {
  "run",
  "--allow-read",
  "examples/cat.ts",
- "--",
  "README.md"
  ],
  stdout: "piped"

std/fs/empty_dir_test.ts

@@ -217,13 +217,12 @@ 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);

std/fs/exists_test.ts

@@ -124,7 +124,6 @@ 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({

std/http/file_server_test.ts

@@ -14,7 +14,6 @@ async function startFileServer(): Promise<void> {
  "--allow-read",
  "--allow-net",
  "http/file_server.ts",
- "--",
  ".",
  "--cors"
  ],
@@ -123,7 +122,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!));

std/installer/mod.ts

@@ -255,7 +255,6 @@ export async function install(
  "run",
  ...grantedPermissions.map(getFlagFromPermission),
  moduleUrl,
- "--",
  ...scriptArgs
  ];
 

std/installer/test.ts

@@ -1,5 +1,5 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-const { run, stat, makeTempDir, remove, env } = Deno;
+const { run, stat, makeTempDir, remove, env, readAll } = Deno;
 
 import { test, runIfMain, TestFunction } from "../testing/mod.ts";
 import { assert, assertEquals } from "../testing/asserts.ts";
@@ -20,7 +20,6 @@ async function startFileServer(): Promise<void> {
  "--allow-read",
  "--allow-net",
  "http/file_server.ts",
- "--",
  ".",
  "--cors"
  ],
@@ -84,11 +83,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 */
@@ -107,10 +106,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
@@ -139,11 +138,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 */
@@ -162,10 +161,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
@@ -193,11 +192,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 */
@@ -216,10 +215,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
@@ -228,17 +227,57 @@ 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");
@@ -276,9 +315,7 @@ 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(
@@ -320,9 +357,7 @@ 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(
@@ -350,7 +385,9 @@ 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");
@@ -367,7 +404,6 @@ 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();
@@ -391,11 +427,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 */
@@ -414,10 +450,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

std/manual.md

@@ -394,7 +394,6 @@ const p = Deno.run({
  "run",
  "--allow-read",
  "https://deno.land/std/examples/cat.ts",
- "--",
  ...fileNames
  ],
  stdout: "piped",