Skip to content

feat: add git-bash support to shell integration#208960

Merged
Tyriar merged 3 commits intomicrosoft:mainfrom
cpendery:feat/git-bash-shell-integration
Mar 28, 2024
Merged

feat: add git-bash support to shell integration#208960
Tyriar merged 3 commits intomicrosoft:mainfrom
cpendery:feat/git-bash-shell-integration

Conversation

@cpendery
Copy link
Member

@cpendery cpendery commented Mar 27, 2024 •
edited by Tyriar
Loading

Adds support for shell integration to Git Bash when using the default prompt & includes the following tweaks

  • Increase MaxCheckLineCount to 10 since bash's commands often output responses in the 6-8 lines range for built in commands
  • Add a bash prompt for $ prompts
  • fix areZshBashLoginArgs to check for --login instead of powershell's -login
  • update areZshBashLoginArgs to ignore any interactive args since the default args for the setting "terminal.integrated.defaultProfile.windows": "Git Bash" is --login -i.
  • update shellIntegration-bash.sh to avoid sending a command complete OSC when the shell first starts up as this leads to bad marker placements on new Git Bash shells.
gitBashShellIntegration.mp4

Closes #143769

@Tyriar Tyriar added this to the April 2024 milestone Mar 27, 2024
Tyriar
Tyriar reviewed Mar 27, 2024
View reviewed changes
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Very nice 👏

How confident are you in the reliability of this happening automatically? We may want to make it opt-in via a setting for git bash for a month depending on the thinking here.

@cpendery
Copy link
Member Author

cpendery commented Mar 28, 2024

Very nice 👏

How confident are you in the reliability of this happening automatically? We may want to make it opt-in via a setting for git bash for a month depending on the thinking here.

I think it makes sense to make it opt in. The sticky scroll isn't reliably working (it works roughly 80% of the time when testing) and if users have a different prompt (not $ terminated) configured, they probably don't want it enabled.

Tyriar
Tyriar previously approved these changes Mar 28, 2024
View reviewed changes
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Looks pretty solid to me, let's enable it by default for April in Insiders and if there are reports about something being broken I can add an opt-in setting for it.

@Tyriar Tyriar mentioned this pull request Mar 28, 2024
Closed
Tyriar
Tyriar previously approved these changes Mar 28, 2024
View reviewed changes
lramos15
lramos15 previously approved these changes Mar 28, 2024
View reviewed changes
@cpendery cpendery dismissed stale reviews from lramos15 and Tyriar via 719c240 March 28, 2024 17:03
@cpendery
Copy link
Member Author

cpendery commented Mar 28, 2024

Looks pretty solid to me, let's enable it by default for April in Insiders and if there are reports about something being broken I can add an opt-in setting for it.

Ah sorry I missed this before I just pushed the opt-in setting. I reverted that change

@Tyriar Tyriar merged commit ff1fc11 into microsoft:main Mar 28, 2024
@IllusionMH
Copy link
Contributor

IllusionMH commented Mar 29, 2024

First of all, @cpendery, thanks for PR! Eager to check it when it will be in Insiders.

Another question: should terminal.integrated.shellIntegration.enabled description be updated to include Git Bash?

vscode/src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts

Line 582 in 960217f

markdownDescription: localize('terminal.integrated.shellIntegration.enabled', "Determines whether or not shell integration is auto-injected to support features like enhanced command tracking and current working directory detection. \n\nShell integration works by injecting the shell with a startup script. The script gives VS Code insight into what is happening within the terminal.\n\nSupported shells:\n\n- Linux/macOS: bash, fish, pwsh, zsh\n - Windows: pwsh\n\nThis setting applies only when terminals are created, so you will need to restart your terminals for it to take effect.\n\n Note that the script injection may not work if you have custom arguments defined in the terminal profile, have enabled {1}, have a [complex bash `PROMPT_COMMAND`](https://code.visualstudio.com/docs/editor/integrated-terminal#_complex-bash-promptcommand), or other unsupported setup. To disable decorations, see {0}", '`#terminal.integrated.shellIntegrations.decorationsEnabled#`', '`#editor.accessibilitySupport#`'),

@Tyriar
Copy link
Member

Tyriar commented Mar 29, 2024

@IllusionMH yep, thanks for pointing that out. I'll fix it now

@Tyriar
Copy link
Member

Tyriar commented Mar 29, 2024

#209107

@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Tyriar Tyriar Tyriar approved these changes

@lramos15 lramos15 lramos15 left review comments

+1 more reviewer

@karrtikr karrtikr karrtikr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Tyriar Tyriar

Labels

None yet

Projects

None yet

Milestone

April 2024

Development

Successfully merging this pull request may close these issues.

Support Git Bash shell integration automatic injection

5 participants

@cpendery @IllusionMH @Tyriar @lramos15 @karrtikr