Skip to content

Commit

Permalink
Fix shell-integration-features being ignored with manual shell integr…
Browse files Browse the repository at this point in the history
…ation (#5048)

## Descriptions

The code was short-circuiting the shell integration setup when
`shell-integration = none`, which prevented the feature environment
variables from being set. These environment variables are needed even
for manual shell integration to work properly.

## Changes

- Extracted feature environment variables setup into a separate
`setup_features` function

- Modified the shell integration initialization to ensure features are
set up even when `shell-integration = none`

<img width="1126" alt="image"
src="https://github.com/user-attachments/assets/ceeb33f5-26ee-4a3b-a6d5-eed57848c96c"
/>


Fixes #5046
  • Loading branch information
mitchellh authored Jan 20, 2025
2 parents c3ef4d2 + ccd6fd2 commit 8ada93d
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 55 deletions.
4 changes: 3 additions & 1 deletion src/config/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,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

0 comments on commit 8ada93d

Please sign in to comment.