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

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

Merged
merged 3 commits into from
Mar 28, 2024

Conversation

cpendery
Copy link
Member

@cpendery cpendery commented Mar 27, 2024

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

Signed-off-by: Chapman Pendery <cpendery@microsoft.com>
@Tyriar Tyriar added this to the April 2024 milestone Mar 27, 2024
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

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
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 previously approved these changes Mar 28, 2024
lramos15
lramos15 previously approved these changes Mar 28, 2024
@cpendery cpendery dismissed stale reviews from lramos15 and Tyriar via 719c240 March 28, 2024 17:03
@cpendery cpendery force-pushed the feat/git-bash-shell-integration branch from 719c240 to 5703cdc Compare March 28, 2024 17:05
@cpendery
Copy link
Member Author

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
9 checks passed
@IllusionMH
Copy link
Contributor

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?

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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Git Bash shell integration automatic injection
5 participants