mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-05-24 05:40:15 +00:00
termio: run Windows shell commands without a cmd.exe wrapper (#12389)
On Windows the configured shell was always executed as `cmd.exe /C <shell>`. That inserts a cmd.exe even for simple values like `command = wsl ~` or `command = pwsh -NoLogo`, producing two processes where one would do. Two concrete side effects: An extra cmd.exe appears in every Windows terminal's process tree (visible in Task Manager / process listings), two processes per surface where only one is the user's shell. cmd.exe state set by AutoRun (`HKCU\Software\Microsoft\Command Processor\AutoRun`, used commonly for DOSKEY aliases or `cd` in `init.cmd`) lives in the wrapping cmd process, not in the user's shell. Since AutoRun state like DOSKEY aliases is per-process, the user's aliases don't reach the shell they actually interact with. Run the shell value directly instead. If it contains whitespace, split on whitespace into argv. A bare `cmd.exe` is resolved via `%COMSPEC%` (the documented path to the current command processor). Other bare values are left to PATH resolution in `Command.startWindows` (#12387). The simple whitespace split does not honor Windows CLI quoting rules; users who need quoted arguments should use the direct command form, which takes an argv array as-is. For the common case (`wsl ~`, `pwsh -NoLogo`, `cmd.exe /k init.cmd`, etc.) this covers the shapes users actually write today. Also skips the termios focus timer on Windows in `focusGained`, since Windows has no termios -- the callback was arming a timer whose tick did nothing and just added noise. --- AI usage disclosure: developed with Claude Code (Claude Opus 4.7). Claude drafted the implementation based on my design direction -- I picked which pieces belong in this PR (drop the cmd wrapper, use `%COMSPEC%`, skip the termios focus timer) and which belong in sibling PRs. I reviewed each diff and validated it with a Windows GNU-ABI smoke build before pushing. Part of the Win32 apprt upstreaming series (see discussion #2563 / mattn/ghostty#1).
This commit is contained in:
@@ -237,6 +237,9 @@ pub fn focusGained(
|
||||
assert(td.backend == .exec);
|
||||
const execdata = &td.backend.exec;
|
||||
|
||||
// Windows has no termios, so there is nothing to poll.
|
||||
if (comptime builtin.os.tag == .windows) return;
|
||||
|
||||
if (!focused) {
|
||||
// Flag the timer to end on the next iteration. This is
|
||||
// a lot cheaper than doing full timer cancellation.
|
||||
@@ -1552,26 +1555,39 @@ fn execCommand(
|
||||
defer args.deinit(alloc);
|
||||
|
||||
if (comptime builtin.os.tag == .windows) {
|
||||
// We run our shell wrapped in `cmd.exe` so that we don't have
|
||||
// to parse the command line ourselves if it has arguments.
|
||||
|
||||
// On Windows we run the shell value directly rather than
|
||||
// wrapping in `cmd.exe /C <shell>`. An intermediate cmd
|
||||
// process is wasteful for the common case (`wsl ~`,
|
||||
// `pwsh -NoLogo`, etc.) and has visible side effects
|
||||
// (extra process in the tree, per-process cmd AutoRun
|
||||
// state not reaching the user's actual shell).
|
||||
//
|
||||
// Values with arguments are split on whitespace. This
|
||||
// does not honor Windows CLI quoting rules; users who
|
||||
// need quoted arguments should use the direct command
|
||||
// form, which takes an argv array as-is.
|
||||
//
|
||||
// Note we don't free any of the memory below since it is
|
||||
// allocated in the arena.
|
||||
const windir = std.process.getEnvVarOwned(
|
||||
alloc,
|
||||
"WINDIR",
|
||||
) catch |err| {
|
||||
log.warn("failed to get WINDIR, cannot run shell command err={}", .{err});
|
||||
return error.SystemError;
|
||||
};
|
||||
const cmd = try std.fs.path.joinZ(alloc, &[_][]const u8{
|
||||
windir,
|
||||
"System32",
|
||||
"cmd.exe",
|
||||
});
|
||||
|
||||
try args.append(alloc, cmd);
|
||||
try args.append(alloc, "/C");
|
||||
if (std.mem.indexOfAny(u8, v, " \t") == null) {
|
||||
// No arguments. If the shell is literally "cmd.exe"
|
||||
// (the default), resolve via %COMSPEC% which is the
|
||||
// documented path to the current command processor.
|
||||
// Other values are passed as-is and resolved by
|
||||
// `internal_os.path.expand` in Command.startWindows.
|
||||
const argv0 = if (std.ascii.eqlIgnoreCase(v, "cmd.exe"))
|
||||
std.process.getEnvVarOwned(alloc, "COMSPEC") catch
|
||||
try alloc.dupe(u8, v)
|
||||
else
|
||||
try alloc.dupe(u8, v);
|
||||
try args.append(alloc, try alloc.dupeZ(u8, argv0));
|
||||
} else {
|
||||
var it = std.mem.tokenizeAny(u8, v, " \t");
|
||||
while (it.next()) |tok| {
|
||||
try args.append(alloc, try alloc.dupeZ(u8, tok));
|
||||
}
|
||||
}
|
||||
break :shell try args.toOwnedSlice(alloc);
|
||||
} else {
|
||||
// We run our shell wrapped in `/bin/sh` so that we don't have
|
||||
// to parse the command line ourselves if it has arguments.
|
||||
@@ -1759,3 +1775,84 @@ test "execCommand: direct command, config freed" {
|
||||
try testing.expectEqualStrings(result[0], "foo");
|
||||
try testing.expectEqualStrings(result[1], "bar baz");
|
||||
}
|
||||
|
||||
test "execCommand windows: bare cmd.exe resolves via COMSPEC" {
|
||||
if (comptime builtin.os.tag != .windows) return error.SkipZigTest;
|
||||
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
const result = try execCommand(alloc, .{ .shell = "cmd.exe" }, struct {
|
||||
fn get(_: Allocator) !PasswdEntry {
|
||||
return .{};
|
||||
}
|
||||
});
|
||||
|
||||
try testing.expectEqual(1, result.len);
|
||||
|
||||
// Expect COMSPEC if available, otherwise the documented fallback.
|
||||
const expected = std.process.getEnvVarOwned(alloc, "COMSPEC") catch
|
||||
try alloc.dupe(u8, "C:\\Windows\\System32\\cmd.exe");
|
||||
try testing.expectEqualStrings(expected, result[0]);
|
||||
}
|
||||
|
||||
test "execCommand windows: bare non-cmd shell is passed through" {
|
||||
if (comptime builtin.os.tag != .windows) return error.SkipZigTest;
|
||||
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
const result = try execCommand(alloc, .{ .shell = "pwsh.exe" }, struct {
|
||||
fn get(_: Allocator) !PasswdEntry {
|
||||
return .{};
|
||||
}
|
||||
});
|
||||
|
||||
try testing.expectEqual(1, result.len);
|
||||
try testing.expectEqualStrings("pwsh.exe", result[0]);
|
||||
}
|
||||
|
||||
test "execCommand windows: shell with args is split on whitespace" {
|
||||
if (comptime builtin.os.tag != .windows) return error.SkipZigTest;
|
||||
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
const result = try execCommand(alloc, .{ .shell = "wsl ~" }, struct {
|
||||
fn get(_: Allocator) !PasswdEntry {
|
||||
return .{};
|
||||
}
|
||||
});
|
||||
|
||||
try testing.expectEqual(2, result.len);
|
||||
try testing.expectEqualStrings("wsl", result[0]);
|
||||
try testing.expectEqualStrings("~", result[1]);
|
||||
}
|
||||
|
||||
test "execCommand windows: direct command is passed through unchanged" {
|
||||
if (comptime builtin.os.tag != .windows) return error.SkipZigTest;
|
||||
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
const result = try execCommand(alloc, .{ .direct = &.{
|
||||
"C:\\tools\\foo.exe",
|
||||
"arg with spaces",
|
||||
} }, struct {
|
||||
fn get(_: Allocator) !PasswdEntry {
|
||||
return .{};
|
||||
}
|
||||
});
|
||||
|
||||
try testing.expectEqual(2, result.len);
|
||||
try testing.expectEqualStrings("C:\\tools\\foo.exe", result[0]);
|
||||
try testing.expectEqualStrings("arg with spaces", result[1]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user