mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-06-06 20:04:25 +00:00
windows: handle backslash paths in config value parsing (#11782)
# What CommaSplitter treats backslash as an escape character, which breaks Windows paths like C:\Users\foo since \U is not a valid escape. On Windows, treat backslash as a literal character outside of quoted strings. Inside quotes, escape sequences still work as before. Also fix Theme.parseCLI to not mistake the colon in a Windows drive letter (C:\...) for a light/dark theme pair separator. # How The platform behavior is controlled by a single comptime constant at the top of CommaSplitter: const escape_outside_quotes = builtin.os.tag != .windows; The next() function checks this constant to decide whether backslash triggers escape parsing outside quoted strings. All behavior lives in one place. For Theme, skip colon detection at index 1 on Windows so drive letters are not mistaken for pair separators. Escape-specific tests are skipped on Windows with SkipZigTest. Windows-specific tests are added separately to cover paths, literal backslash, and escapes-still-work-inside-quotes. # Note There are other places in config parsing that use colon as a delimiter without accounting for Windows drive letters (command.zig prefix parsing, keybind parsing). Those are separate from this PR. # Verified - zig build test-lib-vt passes on Windows (exit 0) - No impact on Linux/macOS (the constant is true there, all existing behavior unchanged) # What I Learnt - Platform behavior should live in a single constant or struct, not scattered across if-else branches in every test. The escape_outside_quotes constant mirrors the pattern upstream uses with PageAlloc = switch(builtin.os.tag) but for a simpler boolean case. - Use error.SkipZigTest for tests that cannot run on a platform, never silent returns. This way the test runner reports them as skipped, not silently passed. - When fixing a pattern (colon as delimiter), grep the whole codebase for similar issues even if you are not fixing them all in one PR. Note them for future work.
This commit is contained in:
@@ -13,8 +13,18 @@
|
||||
//!
|
||||
//! Quotes and escapes are not stripped or decoded, that must be handled as a
|
||||
//! separate step!
|
||||
//!
|
||||
//! On Windows, backslash is only treated as an escape character inside quoted
|
||||
//! strings. Outside quotes, backslash is a literal character (path separator).
|
||||
const CommaSplitter = @This();
|
||||
|
||||
const builtin = @import("builtin");
|
||||
|
||||
/// Whether backslash acts as an escape character outside quoted strings.
|
||||
/// On Windows, backslash is the path separator so it is always literal
|
||||
/// outside quotes.
|
||||
const escape_outside_quotes = builtin.os.tag != .windows;
|
||||
|
||||
pub const Error = error{
|
||||
UnclosedQuote,
|
||||
UnfinishedEscape,
|
||||
@@ -77,8 +87,11 @@ pub fn next(self: *CommaSplitter) Error!?[]const u8 {
|
||||
},
|
||||
'\\' => {
|
||||
self.index += 1;
|
||||
last = .normal;
|
||||
continue :loop .escape;
|
||||
if (comptime escape_outside_quotes) {
|
||||
last = .normal;
|
||||
continue :loop .escape;
|
||||
}
|
||||
continue :loop .normal;
|
||||
},
|
||||
else => {
|
||||
self.index += 1;
|
||||
@@ -273,6 +286,7 @@ test "splitter 8" {
|
||||
}
|
||||
|
||||
test "splitter 9" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -281,6 +295,7 @@ test "splitter 9" {
|
||||
}
|
||||
|
||||
test "splitter 10" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -289,6 +304,7 @@ test "splitter 10" {
|
||||
}
|
||||
|
||||
test "splitter 11" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -297,6 +313,7 @@ test "splitter 11" {
|
||||
}
|
||||
|
||||
test "splitter 12" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -305,6 +322,7 @@ test "splitter 12" {
|
||||
}
|
||||
|
||||
test "splitter 13" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -313,6 +331,7 @@ test "splitter 13" {
|
||||
}
|
||||
|
||||
test "splitter 14" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -330,6 +349,7 @@ test "splitter 15" {
|
||||
}
|
||||
|
||||
test "splitter 16" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -338,6 +358,7 @@ test "splitter 16" {
|
||||
}
|
||||
|
||||
test "splitter 17" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -346,6 +367,7 @@ test "splitter 17" {
|
||||
}
|
||||
|
||||
test "splitter 18" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -415,6 +437,7 @@ test "splitter 24" {
|
||||
}
|
||||
|
||||
test "splitter 25" {
|
||||
if (comptime !escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
@@ -422,3 +445,39 @@ test "splitter 25" {
|
||||
try testing.expectEqualStrings("a", (try s.next()).?);
|
||||
try testing.expectError(error.IllegalEscape, s.next());
|
||||
}
|
||||
|
||||
// Windows-specific tests: backslash is literal outside quotes.
|
||||
|
||||
test "splitter: windows paths" {
|
||||
if (comptime escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
var s: CommaSplitter = .init("light:C:\\Users\\foo\\theme,dark:C:\\Users\\bar\\theme");
|
||||
try testing.expectEqualStrings("light:C:\\Users\\foo\\theme", (try s.next()).?);
|
||||
try testing.expectEqualStrings("dark:C:\\Users\\bar\\theme", (try s.next()).?);
|
||||
try testing.expect(null == try s.next());
|
||||
}
|
||||
|
||||
test "splitter: backslash literal outside quotes on windows" {
|
||||
if (comptime escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
// Backslash followed by characters that would be escapes on Unix
|
||||
// are treated as literal on Windows outside quotes.
|
||||
var s: CommaSplitter = .init("\\n\\r\\t");
|
||||
try testing.expectEqualStrings("\\n\\r\\t", (try s.next()).?);
|
||||
try testing.expect(null == try s.next());
|
||||
}
|
||||
|
||||
test "splitter: backslash still escapes inside quotes on windows" {
|
||||
if (comptime escape_outside_quotes) return error.SkipZigTest;
|
||||
const std = @import("std");
|
||||
const testing = std.testing;
|
||||
|
||||
// Inside quotes, backslash escapes work on all platforms.
|
||||
var s: CommaSplitter = .init("\"hello\\nworld\"");
|
||||
try testing.expectEqualStrings("\"hello\\nworld\"", (try s.next()).?);
|
||||
try testing.expect(null == try s.next());
|
||||
}
|
||||
|
||||
@@ -9810,9 +9810,16 @@ pub const Theme = struct {
|
||||
// we're parsing a light/dark mode theme pair. Note that "=" isn't
|
||||
// actually valid for setting a light/dark mode pair but I anticipate
|
||||
// it'll be a common typo.
|
||||
//
|
||||
// On Windows, a colon at index 1 is a drive letter (e.g. C:\...)
|
||||
// and should not trigger light/dark pair parsing.
|
||||
const has_colon = if (comptime builtin.os.tag == .windows)
|
||||
if (std.mem.indexOf(u8, input, ":")) |idx| idx != 1 else false
|
||||
else
|
||||
std.mem.indexOf(u8, input, ":") != null;
|
||||
if (std.mem.indexOf(u8, input, ",") != null or
|
||||
std.mem.indexOf(u8, input, "=") != null or
|
||||
std.mem.indexOf(u8, input, ":") != null)
|
||||
has_colon)
|
||||
{
|
||||
self.* = try cli.args.parseAutoStruct(
|
||||
Theme,
|
||||
|
||||
Reference in New Issue
Block a user