Skip to content

core: don't force shell integration unless we're running that shell#12390

Open
jcollie wants to merge 1 commit intoghostty-org:mainfrom
jcollie:nushell-new-window-command
Open

core: don't force shell integration unless we're running that shell#12390
jcollie wants to merge 1 commit intoghostty-org:mainfrom
jcollie:nushell-new-window-command

Conversation

@jcollie
Copy link
Copy Markdown
Member

@jcollie jcollie commented Apr 23, 2026

If you bypassed shell detection and forced a specific shell integration with shell-integration=<shell> the bash and nushell integrations would break commands that were specified with +new-window -e <command> (or a similar mechanism) because they modify the command in shell-specific ways and the new command would fail with an "unknown flag" error or similar.

This PR fixes that by only permitting shell integration if the command matches the type of shell integration. That means that shell-integration=nushell only works if argv[0] ends with nu, etc. Generally this should not matter unless the shell executable was renamed for some reason and the shell detection failed.

Fixes #12378

If you bypassed shell detection and forced a specific shell integration
with `shell-integration=<shell>` the `bash` and `nushell` integrations
would break commands that were specified with `+new-window -e
<command>` (or a similar mechanism) because they modify the command
in shell-specific ways and the new command would fail with an "unknown
flag" error or similar.

This PR fixes that by only permitting shell integration if the
command matches the type of shell integration. That means that
`shell-integration=nushell` only works if `argv[0]` ends with `nu`, etc.
Generally this should not matter unless the shell executable was renamed
for some reason and the shell detection failed.

Fixes ghostty-org#12378
@jcollie jcollie requested a review from a team as a code owner April 23, 2026 05:43
@mitchellh mitchellh requested a review from jparise April 23, 2026 13:36
@mitchellh mitchellh added this to the 1.3.2 milestone Apr 23, 2026
Copy link
Copy Markdown
Contributor

@jparise jparise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't look in detail right now (temporarily internet-less), but my initial impression is that this approach isn't quite right.

  • The purpose of "forced" shell integration is to ignore the command-based detection, generally for cases when the command is named something else (for reasons we don't know or necessarily need to understand). With the proposed approach, we're basically just using a hinted "detect" path.
  • If we don't want shell integration in the +new-window path, I think we should skip calling shell_integration.setup() or pass a somehow different command into it. (This is the part I can't research while on mobile, so sorry if I'm missing something.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Setting shell-integration = nushell breaks ghostty +new-window -e on Linux

3 participants