Skip to content

refactor: remove folder/workspace from vsCodeCliArgs #4932

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

Merged
merged 14 commits into from
Mar 2, 2022
Merged
28 changes: 2 additions & 26 deletions src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,7 @@ import { promises as fs } from "fs"
import yaml from "js-yaml"
import * as os from "os"
import * as path from "path"
import {
canConnect,
generateCertificate,
generatePassword,
humanPath,
paths,
isNodeJSErrnoException,
isFile,
} from "./util"
import { canConnect, generateCertificate, generatePassword, humanPath, paths, isNodeJSErrnoException } from "./util"

const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc")

Expand Down Expand Up @@ -448,7 +440,7 @@ export interface DefaultedArgs extends ConfigArgs {
"extensions-dir": string
"user-data-dir": string
/* Positional arguments. */
_: []
_: string[]
}

/**
Expand Down Expand Up @@ -770,25 +762,9 @@ export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Prom
* Convert our arguments to VS Code server arguments.
*/
export const toVsCodeArgs = async (args: DefaultedArgs): Promise<CodeServerLib.ServerParsedArgs> => {
let workspace = ""
let folder = ""
if (args._.length) {
const lastEntry = path.resolve(args._[args._.length - 1])
const entryIsFile = await isFile(lastEntry)
if (entryIsFile && path.extname(lastEntry) === ".code-workspace") {
workspace = lastEntry
} else if (!entryIsFile) {
folder = lastEntry
}
// Otherwise it is a regular file. Spawning VS Code with a file is not yet
// supported but it can be done separately after code-server spawns.
}

return {
"connection-token": "0000",
...args,
workspace,
folder,
"accept-server-license-terms": true,
/** Type casting. */
help: !!args.help,
Expand Down
51 changes: 34 additions & 17 deletions src/node/routes/vscode.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { logger } from "@coder/logger"
import * as express from "express"
import * as path from "path"
import { WebsocketRequest } from "../../../typings/pluginapi"
import { logError } from "../../common/util"
import { toVsCodeArgs } from "../cli"
import { isDevMode } from "../constants"
import { authenticated, ensureAuthenticated, redirect, self } from "../http"
import { SocketProxyProvider } from "../socket"
import { loadAMDModule } from "../util"
import { isFile, loadAMDModule } from "../util"
import { Router as WsRouter } from "../wsRouter"
import { errorHandler } from "./errors"

Expand All @@ -25,6 +26,9 @@ export class CodeServerRouteWrapper {

private $root: express.Handler = async (req, res, next) => {
const isAuthenticated = await authenticated(req)
const NO_FOLDER_OR_WORKSPACE_QUERY = !req.query.folder && !req.query.workspace
// Ew means the workspace was closed so clear the last folder/workspace.
const FOLDER_OR_WORKSPACE_WAS_CLOSED = req.query.ew

if (!isAuthenticated) {
const to = self(req)
Expand All @@ -33,25 +37,38 @@ export class CodeServerRouteWrapper {
})
}

const { query } = await req.settings.read()
if (query) {
// Ew means the workspace was closed so clear the last folder/workspace.
if (req.query.ew) {
delete query.folder
delete query.workspace
}
if (NO_FOLDER_OR_WORKSPACE_QUERY && !FOLDER_OR_WORKSPACE_WAS_CLOSED) {
const settings = await req.settings.read()
const lastOpened = settings.query || {}
// This flag disables the last opened behavior
const IGNORE_LAST_OPENED = req.args["ignore-last-opened"]
const HAS_LAST_OPENED_FOLDER_OR_WORKSPACE = lastOpened.folder || lastOpened.workspace
const HAS_FOLDER_OR_WORKSPACE_FROM_CLI = req.args._.length > 0
const to = self(req)

let folder = undefined
let workspace = undefined

// Redirect to the last folder/workspace if nothing else is opened.
if (
!req.query.folder &&
!req.query.workspace &&
(query.folder || query.workspace) &&
!req.args["ignore-last-opened"] // This flag disables this behavior.
) {
const to = self(req)
if (HAS_LAST_OPENED_FOLDER_OR_WORKSPACE && !IGNORE_LAST_OPENED) {
folder = lastOpened.folder
workspace = lastOpened.workspace
} else if (HAS_FOLDER_OR_WORKSPACE_FROM_CLI) {
const lastEntry = path.resolve(req.args._[req.args._.length - 1])
const entryIsFile = await isFile(lastEntry)
const IS_WORKSPACE_FILE = entryIsFile && path.extname(lastEntry) === ".code-workspace"

if (IS_WORKSPACE_FILE) {
workspace = lastEntry
} else if (!entryIsFile) {
folder = lastEntry
}
}

if (folder || workspace) {
return redirect(req, res, to, {
folder: query.folder,
workspace: query.workspace,
folder,
workspace,
})
}
}
Expand Down
25 changes: 0 additions & 25 deletions test/unit/node/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,29 +726,6 @@ describe("toVsCodeArgs", () => {
it("should convert empty args", async () => {
expect(await toVsCodeArgs(await setDefaults(parse([])))).toStrictEqual({
...vscodeDefaults,
folder: "",
workspace: "",
})
})

it("should convert with workspace", async () => {
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(workspace, "foobar")
expect(await toVsCodeArgs(await setDefaults(parse([workspace])))).toStrictEqual({
...vscodeDefaults,
workspace,
folder: "",
_: [workspace],
})
})

it("should convert with folder", async () => {
const folder = await tmpdir(testName)
expect(await toVsCodeArgs(await setDefaults(parse([folder])))).toStrictEqual({
...vscodeDefaults,
folder,
workspace: "",
_: [folder],
})
})

Expand All @@ -757,8 +734,6 @@ describe("toVsCodeArgs", () => {
await fs.writeFile(file, "foobar")
expect(await toVsCodeArgs(await setDefaults(parse([file])))).toStrictEqual({
...vscodeDefaults,
folder: "",
workspace: "",
_: [file],
})
})
Expand Down
91 changes: 34 additions & 57 deletions test/unit/node/routes/vscode.test.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
import { promises as fs } from "fs"
import { Response } from "node-fetch"
import * as path from "path"
import { clean, tmpdir } from "../../../utils/helpers"
import * as httpserver from "../../../utils/httpserver"
import * as integration from "../../../utils/integration"

interface WorkbenchConfig {
folderUri?: {
path: string
}
workspaceUri?: {
path: string
}
}

describe("vscode", () => {
let codeServer: httpserver.HttpServer | undefined

Expand All @@ -39,7 +29,7 @@ describe("vscode", () => {
expect(resp.status).toBe(200)
const html = await resp.text()
const url = new URL(resp.url) // Check there were no redirections.
expect(url.pathname + decodeURIComponent(url.search)).toBe(route)
expect(url.pathname + url.search).toBe(route)
switch (route) {
case "/":
case "/vscode/":
Expand All @@ -52,59 +42,33 @@ describe("vscode", () => {
}
})

/**
* Get the workbench config from the provided response.
*/
const getConfig = async (resp: Response): Promise<WorkbenchConfig> => {
expect(resp.status).toBe(200)
const html = await resp.text()
const match = html.match(/<meta id="vscode-workbench-web-configuration" data-settings="(.+)">/)
if (!match || !match[1]) {
throw new Error("Unable to find workbench configuration")
}
const config = match[1].replace(/&quot;/g, '"')
try {
return JSON.parse(config)
} catch (error) {
console.error("Failed to parse workbench configuration", config)
throw error
}
}

it("should have no default folder or workspace", async () => {
codeServer = await integration.setup(["--auth=none"], "")

const config = await getConfig(await codeServer.fetch("/"))
expect(config.folderUri).toBeUndefined()
expect(config.workspaceUri).toBeUndefined()
})

it("should have a default folder", async () => {
const defaultDir = await tmpdir(testName)
codeServer = await integration.setup(["--auth=none", defaultDir], "")
it("should redirect to the passed in workspace using human-readable query", async () => {
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(workspace, "")
codeServer = await integration.setup(["--auth=none", workspace], "")

// At first it will load the directory provided on the command line.
const config = await getConfig(await codeServer.fetch("/"))
expect(config.folderUri?.path).toBe(defaultDir)
expect(config.workspaceUri).toBeUndefined()
const resp = await codeServer.fetch("/")
const url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(url.search).toBe(`?workspace=${workspace}`)
})

it("should have a default workspace", async () => {
const defaultWorkspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(defaultWorkspace, "")
codeServer = await integration.setup(["--auth=none", defaultWorkspace], "")
it("should redirect to the passed in folder using human-readable query", async () => {
const folder = await tmpdir(testName)
codeServer = await integration.setup(["--auth=none", folder], "")

// At first it will load the workspace provided on the command line.
const config = await getConfig(await codeServer.fetch("/"))
expect(config.folderUri).toBeUndefined()
expect(config.workspaceUri?.path).toBe(defaultWorkspace)
const resp = await codeServer.fetch("/")
const url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(url.search).toBe(`?folder=${folder}`)
})

it("should redirect to last query folder/workspace", async () => {
codeServer = await integration.setup(["--auth=none"], "")

const folder = await tmpdir(testName)
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(workspace, "")
let resp = await codeServer.fetch("/", undefined, {
folder,
workspace,
Expand All @@ -118,21 +82,32 @@ describe("vscode", () => {
resp = await codeServer.fetch(route)
const url = new URL(resp.url)
expect(url.pathname).toBe(route)
expect(decodeURIComponent(url.search)).toBe(`?folder=${folder}&workspace=${workspace}`)
expect(url.search).toBe(`?folder=${folder}&workspace=${workspace}`)
await resp.text()
}

// Closing the folder should stop the redirecting.
resp = await codeServer.fetch("/", undefined, { ew: "true" })
let url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(decodeURIComponent(url.search)).toBe("?ew=true")
expect(url.search).toBe("?ew=true")
await resp.text()

resp = await codeServer.fetch("/")
url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(decodeURIComponent(url.search)).toBe("")
expect(url.search).toBe("")
await resp.text()
})

it("should do nothing when nothing is passed in", async () => {
codeServer = await integration.setup(["--auth=none"], "")

let resp = await codeServer.fetch("/", undefined)

expect(resp.status).toBe(200)
const url = new URL(resp.url)
expect(url.search).toBe("")
await resp.text()
})

Expand All @@ -141,6 +116,8 @@ describe("vscode", () => {

const folder = await tmpdir(testName)
const workspace = path.join(await tmpdir(testName), "test.code-workspace")
await fs.writeFile(workspace, "")

let resp = await codeServer.fetch("/", undefined, {
folder,
workspace,
Expand All @@ -152,7 +129,7 @@ describe("vscode", () => {
resp = await codeServer.fetch("/")
const url = new URL(resp.url)
expect(url.pathname).toBe("/")
expect(decodeURIComponent(url.search)).toBe("")
expect(url.search).toBe("")
await resp.text()
})
})
2 changes: 1 addition & 1 deletion vendor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"postinstall": "./postinstall.sh"
},
"devDependencies": {
"code-oss-dev": "coder/vscode#0fd21e4078ac1dddb26be024f5d4224a4b86da93"
"code-oss-dev": "coder/vscode#bd734e3d9f21b1bce4dabab2514177e90c090ee6"
}
}
4 changes: 2 additions & 2 deletions vendor/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,9 @@ clone-response@^1.0.2:
dependencies:
mimic-response "^1.0.0"

code-oss-dev@coder/vscode#0fd21e4078ac1dddb26be024f5d4224a4b86da93:
code-oss-dev@coder/vscode#bd734e3d9f21b1bce4dabab2514177e90c090ee6:
version "1.63.0"
resolved "https://codeload.github.com/coder/vscode/tar.gz/0fd21e4078ac1dddb26be024f5d4224a4b86da93"
resolved "https://codeload.github.com/coder/vscode/tar.gz/bd734e3d9f21b1bce4dabab2514177e90c090ee6"
dependencies:
"@microsoft/applicationinsights-web" "^2.6.4"
"@parcel/watcher" "2.0.3"
Expand Down