Skip to content
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

Fix shell-integration-features being ignored with manual shell integration #5048

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/config/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1639,7 +1639,9 @@ keybind: Keybinds = .{},
/// The default value is `detect`.
@"shell-integration": ShellIntegration = .detect,

/// Shell integration features to enable if shell integration itself is enabled.
/// Shell integration features to enable. These require our shell integration
/// to be loaded, either automatically via shell-integration or manually.
///
/// The format of this is a list of features to enable separated by commas. If
/// you prefix a feature with `no-` then it is disabled. If you omit a feature,
/// its default value is used, so you must explicitly disable features you don't
Expand Down
6 changes: 5 additions & 1 deletion src/termio/Exec.zig
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,11 @@ const Subprocess = struct {
};

const force: ?shell_integration.Shell = switch (cfg.shell_integration) {
.none => break :shell .{ null, default_shell_command },
.none => {
// Even if shell integration is none, we still want to set up the feature env vars
try shell_integration.setupFeatures(&env, cfg.shell_integration_features);
break :shell .{ null, default_shell_command };
},
.detect => null,
.bash => .bash,
.elvish => .elvish,
Expand Down
164 changes: 111 additions & 53 deletions src/termio/shell_integration.zig
Original file line number Diff line number Diff line change
Expand Up @@ -58,67 +58,73 @@ pub fn setup(
break :exe std.fs.path.basename(command[0..idx]);
};

const result: ShellIntegration = shell: {
if (std.mem.eql(u8, "bash", exe)) {
// Apple distributes their own patched version of Bash 3.2
// on macOS that disables the ENV-based POSIX startup path.
// This means we're unable to perform our automatic shell
// integration sequence in this specific environment.
//
// If we're running "/bin/bash" on Darwin, we can assume
// we're using Apple's Bash because /bin is non-writable
// on modern macOS due to System Integrity Protection.
if (comptime builtin.target.isDarwin()) {
if (std.mem.eql(u8, "/bin/bash", command)) {
return null;
}
}
const result = try setupShell(alloc_arena, resource_dir, command, env, exe);

const new_command = try setupBash(
alloc_arena,
command,
resource_dir,
env,
) orelse return null;
break :shell .{
.shell = .bash,
.command = new_command,
};
}
// Setup our feature env vars
try setupFeatures(env, features);

if (std.mem.eql(u8, "elvish", exe)) {
try setupXdgDataDirs(alloc_arena, resource_dir, env);
break :shell .{
.shell = .elvish,
.command = try alloc_arena.dupe(u8, command),
};
}
return result;
}

if (std.mem.eql(u8, "fish", exe)) {
try setupXdgDataDirs(alloc_arena, resource_dir, env);
break :shell .{
.shell = .fish,
.command = try alloc_arena.dupe(u8, command),
};
fn setupShell(
alloc_arena: Allocator,
resource_dir: []const u8,
command: []const u8,
env: *EnvMap,
exe: []const u8,
) !?ShellIntegration {
if (std.mem.eql(u8, "bash", exe)) {
// Apple distributes their own patched version of Bash 3.2
// on macOS that disables the ENV-based POSIX startup path.
// This means we're unable to perform our automatic shell
// integration sequence in this specific environment.
//
// If we're running "/bin/bash" on Darwin, we can assume
// we're using Apple's Bash because /bin is non-writable
// on modern macOS due to System Integrity Protection.
if (comptime builtin.target.isDarwin()) {
if (std.mem.eql(u8, "/bin/bash", command)) {
return null;
}
}

if (std.mem.eql(u8, "zsh", exe)) {
try setupZsh(resource_dir, env);
break :shell .{
.shell = .zsh,
.command = try alloc_arena.dupe(u8, command),
};
}
const new_command = try setupBash(
alloc_arena,
command,
resource_dir,
env,
) orelse return null;
return .{
.shell = .bash,
.command = new_command,
};
}

return null;
};
if (std.mem.eql(u8, "elvish", exe)) {
try setupXdgDataDirs(alloc_arena, resource_dir, env);
return .{
.shell = .elvish,
.command = try alloc_arena.dupe(u8, command),
};
}

// Setup our feature env vars
if (!features.cursor) try env.put("GHOSTTY_SHELL_INTEGRATION_NO_CURSOR", "1");
if (!features.sudo) try env.put("GHOSTTY_SHELL_INTEGRATION_NO_SUDO", "1");
if (!features.title) try env.put("GHOSTTY_SHELL_INTEGRATION_NO_TITLE", "1");
if (std.mem.eql(u8, "fish", exe)) {
try setupXdgDataDirs(alloc_arena, resource_dir, env);
return .{
.shell = .fish,
.command = try alloc_arena.dupe(u8, command),
};
}

return result;
if (std.mem.eql(u8, "zsh", exe)) {
try setupZsh(resource_dir, env);
return .{
.shell = .zsh,
.command = try alloc_arena.dupe(u8, command),
};
}

return null;
}

test "force shell" {
Expand All @@ -138,6 +144,58 @@ test "force shell" {
}
}

/// Setup shell integration feature environment variables without
/// performing full shell integration setup.
pub fn setupFeatures(
env: *EnvMap,
features: config.ShellIntegrationFeatures,
) !void {
if (!features.cursor) try env.put("GHOSTTY_SHELL_INTEGRATION_NO_CURSOR", "1");
if (!features.sudo) try env.put("GHOSTTY_SHELL_INTEGRATION_NO_SUDO", "1");
if (!features.title) try env.put("GHOSTTY_SHELL_INTEGRATION_NO_TITLE", "1");
}

test "setup features" {
const testing = std.testing;

var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();

// Test: all features enabled (no environment variables should be set)
{
var env = EnvMap.init(alloc);
defer env.deinit();

try setupFeatures(&env, .{ .cursor = true, .sudo = true, .title = true });
try testing.expect(env.get("GHOSTTY_SHELL_INTEGRATION_NO_CURSOR") == null);
try testing.expect(env.get("GHOSTTY_SHELL_INTEGRATION_NO_SUDO") == null);
try testing.expect(env.get("GHOSTTY_SHELL_INTEGRATION_NO_TITLE") == null);
}

// Test: all features disabled
{
var env = EnvMap.init(alloc);
defer env.deinit();

try setupFeatures(&env, .{ .cursor = false, .sudo = false, .title = false });
try testing.expectEqualStrings("1", env.get("GHOSTTY_SHELL_INTEGRATION_NO_CURSOR").?);
try testing.expectEqualStrings("1", env.get("GHOSTTY_SHELL_INTEGRATION_NO_SUDO").?);
try testing.expectEqualStrings("1", env.get("GHOSTTY_SHELL_INTEGRATION_NO_TITLE").?);
}

// Test: mixed features
{
var env = EnvMap.init(alloc);
defer env.deinit();

try setupFeatures(&env, .{ .cursor = false, .sudo = true, .title = false });
try testing.expectEqualStrings("1", env.get("GHOSTTY_SHELL_INTEGRATION_NO_CURSOR").?);
try testing.expect(env.get("GHOSTTY_SHELL_INTEGRATION_NO_SUDO") == null);
try testing.expectEqualStrings("1", env.get("GHOSTTY_SHELL_INTEGRATION_NO_TITLE").?);
}
}

/// Setup the bash automatic shell integration. This works by
/// starting bash in POSIX mode and using the ENV environment
/// variable to load our bash integration script. This prevents
Expand Down