Skip to content

Commit

Permalink
Add support for Windows .cmd and .bat files (#570)
Browse files Browse the repository at this point in the history
## Summary

I use ruff for our embedded python interpreter. This interpreter needs
to be bootstrapped (env-vars) before be able to get called (technical
limit of the environment itself).
This is usually done with `.cmd` file (the modern version of `.bat`) on
windows which is kind of similar to `.sh` on linux.
`.cmd` files are often use on Windows, even vscode itself use it to
start vscode on windows.

Today I noticed that I get a crash when using the `.cmd` interpreter
path.

It seems that node require shell mode (in windows calling `cmd.exe` as
the shell) to be able to call `.cmd` files correctly.
I needed to quote the input filename as well to avoid whitespace issues
which looks like a bug in node itself.

With that PR I got no crashes anymore when the extension try to run ruff
😄 .

Note: The shell mode only get activated when the platform is windows and
the file extension is `.cmd`, so users with regular executables should
not be affected at all.

## Test Plan

Manual testing it locally works great and as expected.

I added a utils-test to check the require shell mode flag.

The PR is related to the changes of #539

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
  • Loading branch information
tonka3000 and dhruvmanila authored Aug 8, 2024
1 parent 30afd91 commit 8a6c9db
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 1 deletion.
16 changes: 15 additions & 1 deletion src/common/server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as fsapi from "fs-extra";
import * as vscode from "vscode";
import { platform } from "os";
import { Disposable, l10n, LanguageStatusSeverity, LogOutputChannel } from "vscode";
import { State } from "vscode-languageclient";
import {
Expand Down Expand Up @@ -44,12 +45,25 @@ export type IInitializationOptions = {
globalSettings: ISettings;
};

/**
* Check if shell mode is required for `execFile`.
*
* The conditions are:
* - Windows OS
* - File extension is `.cmd` or `.bat`
*/
export function execFileShellModeRequired(file: string) {
file = file.toLowerCase();
return platform() === "win32" && (file.endsWith(".cmd") || file.endsWith(".bat"));
}

/**
* Function to execute a command and return the stdout.
*/
function executeFile(file: string, args: string[] = []): Promise<string> {
const shell = execFileShellModeRequired(file);
return new Promise((resolve, reject) => {
execFile(file, args, (error, stdout, stderr) => {
execFile(shell ? `"${file}"` : file, args, { shell }, (error, stdout, stderr) => {
if (error) {
reject(new Error(stderr || error.message));
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/test/helper.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as vscode from "vscode";
import * as path from "path";
import { platform } from "os";

const EXTENSION_ID = "charliermarsh.ruff";

Expand All @@ -26,3 +27,7 @@ export const getDocumentPath = (p: string) => {
export const getDocumentUri = (p: string) => {
return vscode.Uri.file(getDocumentPath(p));
};

export const isWindows = () => {
return platform() === "win32";
};
28 changes: 28 additions & 0 deletions src/test/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as assert from "assert";
import { execFileShellModeRequired } from "../common/server";
import { isWindows } from "./helper";

suite("Utils tests", () => {
test("Check execFile shell mode", () => {
assert.strictEqual(
execFileShellModeRequired("/use/random/python"),
false,
"Shell mode should not be required for unix paths",
);
assert.strictEqual(
execFileShellModeRequired("C:\\random\\python.exe"),
false,
"Shell mode should not be required for .exe files",
);
assert.strictEqual(
execFileShellModeRequired("C:\\random\\python.cmd"),
isWindows(),
"Shell mode should be required for .cmd files",
);
assert.strictEqual(
execFileShellModeRequired("C:\\random\\python.bat"),
isWindows(),
"Shell mode should be required for .bat files",
);
});
});

0 comments on commit 8a6c9db

Please sign in to comment.