Implement cli parser

scripts/build.ts

@@ -330,7 +330,7 @@ class Builder {
       if (server) {
         server.kill()
       }
-      const s = cp.fork(path.join(this.rootPath, "out/node/entry.js"), process.argv.slice(2))
+      const s = cp.fork(path.join(this.rootPath, "out/node/entry.js"), process.argv.slice(3))
       console.log(`[server] spawned process ${s.pid}`)
       s.on("exit", () => console.log(`[server] process ${s.pid} exited`))
       server = s

scripts/vscode.patch

@@ -934,10 +934,10 @@ index 0000000000..56331ff1fc
 +require('../../bootstrap-amd').load('vs/server/entry');
 diff --git a/src/vs/server/ipc.d.ts b/src/vs/server/ipc.d.ts
 new file mode 100644
-index 0000000000..f3e358096f
+index 0000000000..a1047fff86
 --- /dev/null
 +++ b/src/vs/server/ipc.d.ts
-@@ -0,0 +1,102 @@
+@@ -0,0 +1,101 @@
 +/**
 + * External interfaces for integration into code-server over IPC. No vs imports
 + * should be made in this file.
@@ -984,7 +984,6 @@ index 0000000000..f3e358096f
 +	'extra-builtin-extensions-dir'?: string[];
 +
 +	log?: string;
-+	trace?: boolean;
 +	verbose?: boolean;
 +
 +	_: string[];

src/node/cli.ts

@@ -1,44 +1,223 @@
 import * as path from "path"
-import { logger, Level } from "@coder/logger"
+import { field, logger, Level } from "@coder/logger"
 import { Args as VsArgs } from "../../lib/vscode/src/vs/server/ipc"
 import { AuthType } from "./http"
 import { xdgLocalDir } from "./util"
 
+export class Optional<T> {
+  public constructor(public readonly value?: T) {}
+}
+
+export class OptionalString extends Optional<string> {}
+
 export interface Args extends VsArgs {
-  auth?: AuthType
-  "base-path"?: string
-  cert?: string
-  "cert-key"?: string
-  format?: string
-  host?: string
-  json?: boolean
-  open?: boolean
-  port?: string
-  socket?: string
-  version?: boolean
-  _: string[]
+  readonly auth?: AuthType
+  readonly cert?: OptionalString
+  readonly "cert-key"?: string
+  readonly help?: boolean
+  readonly host?: string
+  readonly json?: boolean
+  readonly open?: boolean
+  readonly port?: number
+  readonly socket?: string
+  readonly version?: boolean
+  readonly _: string[]
 }
 
-// TODO: Implement proper CLI parser.
-export const parse = (): Args => {
-  const last = process.argv[process.argv.length - 1]
-  const userDataDir = xdgLocalDir
-  const verbose = process.argv.includes("--verbose")
-  const trace = process.argv.includes("--trace")
-
-  if (verbose || trace) {
-    process.env.LOG_LEVEL = "trace"
-    logger.level = Level.Trace
-  }
-
-  return {
-    "extensions-dir": path.join(userDataDir, "extensions"),
-    "user-data-dir": userDataDir,
-    _: last && !last.startsWith("-") ? [last] : [],
-    json: process.argv.includes("--json"),
-    log: process.env.LOG_LEVEL,
-    trace,
-    verbose,
-    version: process.argv.includes("--version"),
-  }
+interface Option<T> {
+  type: T
+  /**
+   * Short flag for the option.
+   */
+  short?: string
+  /**
+   * Whether the option is a path and should be resolved.
+   */
+  path?: boolean
+  /**
+   * Description of the option. Leave blank to hide the option.
+   */
+  description?: string
+}
+
+type OptionType<T> = T extends boolean
+  ? "boolean"
+  : T extends OptionalString
+  ? typeof OptionalString
+  : T extends AuthType
+  ? typeof AuthType
+  : T extends number
+  ? "number"
+  : T extends string
+  ? "string"
+  : T extends string[]
+  ? "string[]"
+  : "unknown"
+
+type Options<T> = {
+  [P in keyof T]: Option<OptionType<T[P]>>
+}
+
+const options: Options<Required<Args>> = {
+  auth: { type: AuthType, description: "The type of authentication to use." },
+  cert: {
+    type: OptionalString,
+    path: true,
+    description: "Path to certificate. Generated if no path is provided.",
+  },
+  "cert-key": { type: "string", path: true, description: "Path to certificate key when using non-generated cert." },
+  host: { type: "string", description: "Host for the HTTP server." },
+  help: { type: "boolean", short: "h", description: "Show this output." },
+  json: { type: "boolean" },
+  open: { type: "boolean", description: "Open in the browser on startup. Does not work remotely." },
+  port: { type: "number", description: "Port for the HTTP server." },
+  socket: { type: "string", path: true, description: "Path to a socket (host and port will be ignored)." },
+  version: { type: "boolean", short: "v", description: "Display version information." },
+  _: { type: "string[]" },
+
+  "user-data-dir": { type: "string", path: true, description: "Path to the user data directory." },
+  "extensions-dir": { type: "string", path: true, description: "Path to the extensions directory." },
+  "builtin-extensions-dir": { type: "string", path: true },
+  "extra-extensions-dir": { type: "string[]", path: true },
+  "extra-builtin-extensions-dir": { type: "string[]", path: true },
+
+  log: { type: "string" },
+  verbose: { type: "boolean", short: "vvv", description: "Enable verbose logging." },
+}
+
+export const optionDescriptions = (): string[] => {
+  const entries = Object.entries(options).filter(([, v]) => !!v.description)
+  const widths = entries.reduce(
+    (prev, [k, v]) => ({
+      long: k.length > prev.long ? k.length : prev.long,
+      short: v.short && v.short.length > prev.short ? v.short.length : prev.short,
+    }),
+    { short: 0, long: 0 }
+  )
+  return entries.map(
+    ([k, v]) =>
+      `${" ".repeat(widths.short - (v.short ? v.short.length : 0))}${v.short ? `-${v.short}` : " "} --${k}${" ".repeat(
+        widths.long - k.length
+      )} ${v.description}${typeof v.type === "object" ? ` [${Object.values(v.type).join(", ")}]` : ""}`
+  )
+}
+
+export const parse = (argv: string[]): Args => {
+  const args: Args = { _: [] }
+  let ended = false
+
+  for (let i = 0; i < argv.length; ++i) {
+    const arg = argv[i]
+
+    // -- signals the end of option parsing.
+    if (!ended && arg == "--") {
+      ended = true
+      continue
+    }
+
+    // Options start with a dash and require a value if non-boolean.
+    if (!ended && arg.startsWith("-")) {
+      let key: keyof Args | undefined
+      if (arg.startsWith("--")) {
+        key = arg.replace(/^--/, "") as keyof Args
+      } else {
+        const short = arg.replace(/^-/, "")
+        const pair = Object.entries(options).find(([, v]) => v.short === short)
+        if (pair) {
+          key = pair[0] as keyof Args
+        }
+      }
+
+      if (!key || !options[key]) {
+        throw new Error(`Unknown option ${arg}`)
+      }
+
+      const option = options[key]
+      if (option.type === "boolean") {
+        ;(args[key] as boolean) = true
+        continue
+      }
+
+      // A value is only valid if it doesn't look like an option.
+      let value = argv[i + 1] && !argv[i + 1].startsWith("-") ? argv[++i] : undefined
+      if (!value && option.type === OptionalString) {
+        ;(args[key] as OptionalString) = new OptionalString(value)
+        continue
+      } else if (!value) {
+        throw new Error(`${arg} requires a value`)
+      }
+
+      if (option.path) {
+        value = path.resolve(value)
+      }
+
+      switch (option.type) {
+        case "string":
+          ;(args[key] as string) = value
+          break
+        case "string[]":
+          if (!args[key]) {
+            ;(args[key] as string[]) = []
+          }
+          ;(args[key] as string[]).push(value)
+          break
+        case "number":
+          ;(args[key] as number) = parseInt(value, 10)
+          if (isNaN(args[key] as number)) {
+            throw new Error(`${arg} must be a number`)
+          }
+          break
+        case OptionalString:
+          ;(args[key] as OptionalString) = new OptionalString(value)
+          break
+        default: {
+          if (!Object.values(option.type).find((v) => v === value)) {
+            throw new Error(`${arg} valid values: [${Object.values(option.type).join(", ")}]`)
+          }
+          ;(args[key] as string) = value
+          break
+        }
+      }
+
+      continue
+    }
+
+    // Everything else goes into _.
+    args._.push(arg)
+  }
+
+  logger.debug("parsed command line", field("args", args))
+
+  if (process.env.LOG_LEVEL === "trace" || args.verbose) {
+    args.verbose = true
+    args.log = "trace"
+  }
+
+  switch (args.log) {
+    case "trace":
+      logger.level = Level.Trace
+      break
+    case "debug":
+      logger.level = Level.Debug
+      break
+    case "info":
+      logger.level = Level.Info
+      break
+    case "warning":
+      logger.level = Level.Warning
+      break
+    case "error":
+      logger.level = Level.Error
+      break
+  }
+
+  if (!args["user-data-dir"]) {
+    args["user-data-dir"] = xdgLocalDir
+  }
+
+  if (!args["extensions-dir"]) {
+    args["extensions-dir"] = path.join(args["user-data-dir"], "extensions")
+  }
+
+  return args
 }

src/node/entry.ts

@@ -1,7 +1,7 @@
 import { logger } from "@coder/logger"
 import { ApiHttpProvider } from "./api/server"
 import { MainHttpProvider } from "./app/server"
-import { Args, parse } from "./cli"
+import { Args, optionDescriptions, parse } from "./cli"
 import { AuthType, HttpServer } from "./http"
 import { generateCertificate, generatePassword, hash, open } from "./util"
 import { VscodeHttpProvider } from "./vscode/server"
@@ -21,16 +21,15 @@ const main = async (args: Args): Promise<void> => {
   // Spawn the main HTTP server.
   const options = {
     auth,
-    basePath: args["base-path"],
-    cert: args.cert,
+    cert: args.cert ? args.cert.value : undefined,
     certKey: args["cert-key"],
     commit: commit || "development",
     host: args.host || (args.auth === AuthType.Password && typeof args.cert !== "undefined" ? "0.0.0.0" : "localhost"),
     password: originalPassword ? hash(originalPassword) : undefined,
-    port: typeof args.port !== "undefined" ? parseInt(args.port, 10) : 8080,
+    port: typeof args.port !== "undefined" ? args.port : 8080,
     socket: args.socket,
   }
-  if (!options.cert && typeof options.cert !== "undefined") {
+  if (!options.cert && args.cert) {
     const { cert, certKey } = await generateCertificate()
     options.cert = cert
     options.certKey = certKey
@@ -60,7 +59,7 @@ const main = async (args: Args): Promise<void> => {
 
   if (httpServer.protocol === "https") {
     logger.info(
-      args.cert
+      typeof args.cert === "string"
         ? `  - Using provided certificate${args["cert-key"] ? " and key" : ""} for HTTPS`
         : `  - Using generated certificate and key for HTTPS`
     )
@@ -76,8 +75,17 @@ const main = async (args: Args): Promise<void> => {
   }
 }
 
-const args = parse()
-if (args.version) {
+const args = parse(process.argv.slice(2))
+if (args.help) {
+  console.log("code-server", require("../../package.json").version)
+  console.log("")
+  console.log(`Usage: code-server [options] [path]`)
+  console.log("")
+  console.log("Options")
+  optionDescriptions().forEach((description) => {
+    console.log("", description)
+  })
+} else if (args.version) {
   const version = require("../../package.json").version
   if (args.json) {
     console.log({

test/cli.test.ts

@@ -0,0 +1,126 @@
+import * as assert from "assert"
+import * as path from "path"
+import { parse } from "../src/node/cli"
+import { xdgLocalDir } from "../src/node/util"
+
+describe("cli", () => {
+  it("should set defaults", () => {
+    assert.deepEqual(parse([]), {
+      _: [],
+      "extensions-dir": path.join(xdgLocalDir, "extensions"),
+      "user-data-dir": xdgLocalDir,
+    })
+  })
+
+  it("should parse all available options", () => {
+    assert.deepEqual(
+      parse([
+        "--auth",
+        "none",
+        "--extensions-dir",
+        "foo",
+        "--builtin-extensions-dir",
+        "foobar",
+        "--extra-extensions-dir",
+        "nozzle",
+        "1",
+        "--extra-builtin-extensions-dir",
+        "bazzle",
+        "--verbose",
+        "2",
+        "--log",
+        "error",
+        "--help",
+        "--open",
+        "--socket",
+        "mumble",
+        "3",
+        "--user-data-dir",
+        "bar",
+        "--cert",
+        "baz",
+        "--cert-key",
+        "qux",
+        "--version",
+        "--json",
+        "--port",
+        "8081",
+        "--host",
+        "0.0.0.0",
+        "4",
+        "--",
+        "-5",
+        "--6",
+      ]),
+      {
+        _: ["1", "2", "3", "4", "-5", "--6"],
+        auth: "none",
+        "builtin-extensions-dir": path.resolve("foobar"),
+        "cert-key": path.resolve("qux"),
+        cert: {
+          value: path.resolve("baz"),
+        },
+        "extensions-dir": path.resolve("foo"),
+        "extra-builtin-extensions-dir": [path.resolve("bazzle")],
+        "extra-extensions-dir": [path.resolve("nozzle")],
+        help: true,
+        host: "0.0.0.0",
+        json: true,
+        log: "trace",
+        open: true,
+        port: 8081,
+        socket: path.resolve("mumble"),
+        "user-data-dir": path.resolve("bar"),
+        verbose: true,
+        version: true,
+      }
+    )
+  })
+
+  it("should work with short options", () => {
+    assert.deepEqual(parse(["-vvv", "-v"]), {
+      _: [],
+      "extensions-dir": path.join(xdgLocalDir, "extensions"),
+      "user-data-dir": xdgLocalDir,
+      log: "trace",
+      verbose: true,
+      version: true,
+    })
+  })
+
+  it("should error if value isn't provided", () => {
+    assert.throws(() => parse(["--auth"]), /--auth requires a value/)
+  })
+
+  it("should error if number option is invalid", () => {
+    assert.throws(() => parse(["--port", "foo"]), /--port must be a number/)
+    assert.throws(() => parse(["--auth", "invalid"]), /--auth valid values: \[password, none\]/)
+  })
+
+  it("should error if the option doesn't exist", () => {
+    assert.throws(() => parse(["--foo"]), /Unknown option --foo/)
+  })
+
+  it("should not error if the value is optional", () => {
+    assert.deepEqual(parse(["--cert"]), {
+      _: [],
+      "extensions-dir": path.join(xdgLocalDir, "extensions"),
+      "user-data-dir": xdgLocalDir,
+      cert: {
+        value: undefined,
+      },
+    })
+  })
+
+  it("should not allow option-like values", () => {
+    assert.throws(() => parse(["--socket", "--socket-path-value"]), /--socket requires a value/)
+    // If you actually had a path like this you would do this instead:
+    assert.deepEqual(parse(["--socket", "./--socket-path-value"]), {
+      _: [],
+      "extensions-dir": path.join(xdgLocalDir, "extensions"),
+      "user-data-dir": xdgLocalDir,
+      socket: path.resolve("--socket-path-value"),
+    })
+    assert.throws(() => parse(["--cert", "--socket-path-value"]), /Unknown option --socket-path-value/)
+  })
+})