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.

Sign up for GitHub

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

Jump to bottom

fix: place regex in variables on shellIntegration-bash.sh script #221998

Merged
merged 4 commits into from
Jul 21, 2024
Merged

fix: place regex in variables on shellIntegration-bash.sh script #221998

merged 4 commits into from
Jul 21, 2024

Conversation

kapodamy
Copy link
Contributor

@kapodamy kapodamy commented Jul 17, 2024 •
edited
Loading

Place regular expressions in variables before use, fixes #212139 and allow the script run in older shells.

Tyriar
Tyriar reviewed Jul 18, 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.

Is this really how it works and is it compatible with the older versions or does this condition just never pass then? When I run this in my msys terminal it fails:

image

@kapodamy
Copy link
Contributor Author

should be working now, tested on:
GNU bash, version 3.1.23(1)-release (i686-pc-msys) [MINGW32_??-10.0] oldest
and
GNU bash, version 5.2.26(1)-release (x86_64-pc-msys) [MINGW64_NT-10.0-19045] lastest

Tyriar
Tyriar previously approved these changes Jul 18, 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.

Nice, thanks!

@Tyriar Tyriar added this to the July 2024 milestone Jul 18, 2024
@Tyriar Tyriar enabled auto-merge July 18, 2024 16:33
sbatten
sbatten previously approved these changes Jul 18, 2024
View reviewed changes
@Tyriar
Copy link
Member

Tyriar commented Jul 18, 2024

Seems to be failing the tests which likely mean there's a regression on Linux ☹️

auto-merge was automatically disabled July 18, 2024 23:26

Head branch was pushed to by a user without write access

@kapodamy kapodamy dismissed stale reviews from sbatten and Tyriar via 91a1142 July 18, 2024 23:26
@kapodamy kapodamy changed the title escape regex in shellIntegration-bash.sh file fix: place regex in variables on shellIntegration-bash.sh script Jul 18, 2024
Tyriar
Tyriar approved these changes Jul 20, 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.

Thanks!

@Tyriar Tyriar enabled auto-merge July 21, 2024 13:26
@Tyriar Tyriar merged commit 66edca3 into microsoft:main Jul 21, 2024
6 checks passed
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@roblourens roblourens roblourens approved these changes

@Tyriar Tyriar Tyriar approved these changes

@sbatten sbatten sbatten left review comments

Assignees

@Tyriar Tyriar

Labels
None yet
Projects
None yet
Milestone
July 2024
Development

Successfully merging this pull request may close these issues.

syntax error on sellIntegration-bash.sh
4 participants
@kapodamy @Tyriar @roblourens @sbatten