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

parseArgs duplicates collected + aliased arguments #4481

Closed
j0sh opened this issue Mar 13, 2024 · 3 comments · Fixed by #4485 or #4487
Closed

parseArgs duplicates collected + aliased arguments #4481

j0sh opened this issue Mar 13, 2024 · 3 comments · Fixed by #4485 or #4487

Comments

@j0sh
Copy link

j0sh commented Mar 13, 2024

Version: Deno 1.41.2

The parseArgs function from https://deno.land/std/cli/parse_args.ts seems to duplicate collect arguments when an alias is part of the definition.

This affects the file_server with the --header option; this results in HTTP headers being written twice.

Minimal test case to repro:

import { parseArgs } from "https://deno.land/std/cli/parse_args.ts";
import { assertEquals } from "https://deno.land/std/assert/mod.ts";

Deno.test("collect with alias", () => {
  const args = ["--header", "abc", "--header", "def"];
  const parsed = parseArgs(args, {
    collect: ["header"],
    alias: {
      H: "header",
    },
  });
  assertEquals(parsed.header, ["abc", "def"])
})

Output:

$ deno test
running 1 test from ./duplicatedArgs.test.ts
collect with alias ... FAILED (4ms)

 ERRORS 

collect with alias => ./duplicatedArgs.test.ts:4:6
error: AssertionError: Values are not equal.


    [Diff] Actual / Expected


    [
      "abc",
-     "abc",
-     "def",
      "def",
    ]

Removing the alias makes things work as expected.

@iuioiua
Copy link
Contributor

iuioiua commented Mar 13, 2024

This issue belongs in https://github.com/denoland/deno_std

@bartlomieju bartlomieju transferred this issue from denoland/deno Mar 13, 2024
@kt3k
Copy link
Member

kt3k commented Mar 14, 2024

This seems to have started from #4189

The example test case passes with [email protected] (and fails with [email protected]):

import { parseArgs } from "https://deno.land/[email protected]/cli/parse_args.ts";
import { assertEquals } from "https://deno.land/[email protected]/assert/mod.ts";

Deno.test("collect with alias", () => {
  const args = ["--header", "abc", "--header", "def"];
  const parsed = parseArgs(args, {
    collect: ["header"],
    alias: {
      H: "header",
    },
  });
  assertEquals(parsed.header, ["abc", "def"])
})

I'd suggest we should revert #4189

@timreichen
Copy link
Contributor

timreichen commented Mar 14, 2024

I think I found the bug. The aliasMap Set for key contains key itself. On that example header contains header as an alias.

Map(2) {
  "H" => Set(1) { "header" },
  "header" => Set(2) { "H", "header" }
}

This is fixed by filtering out the key.

before:

const set = new Set([key, ...aliases]);
aliases.forEach((alias) => aliasMap.set(alias, set));

after:

aliases.forEach((alias) => aliasMap.set(alias, new Set([key, ...aliases.filter((it) => it !== alias)])));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants