Skip to content

Fix: Changed bash-syntax to fish-syntax in shellIntegration #181637

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

Merged
merged 2 commits into from
May 9, 2023

Conversation

max06
Copy link
Contributor

@max06 max06 commented May 5, 2023

This PR fixes the regression in the shellIntegration.fish-script.
The used bash-syntax for a command substitution ($(...)) doesn't work in fish (expects (...) without $).
Closes #181634

I wasn't able to find tests for this script. I tested it manually in my insiders instance and I can now see exit status markings again. I'd need further instructions if that's not enough.

@max06 max06 marked this pull request as ready for review May 5, 2023 14:19
@max06 max06 force-pushed the max06/issue181634 branch from d9372f6 to 2b88d64 Compare May 5, 2023 19:34
@Tyriar
Copy link
Member

Tyriar commented May 7, 2023

I'm on fish 3.5.1, do you know why it was working for me but not you?

@Tyriar Tyriar added this to the May 2023 milestone May 7, 2023
@max06
Copy link
Contributor Author

max06 commented May 7, 2023

Thanks to you I just noticed my old fish version. I thought I installed it with brew, but I used the debian repo one.

Checking the changelog, it looks like fish added support for the bash-syntax in 3.4.0. Which is great, better compatibility to bash-scripts.

Testing again locally with both fish versions confirms the changelog: Your working version also works here with the newer fish version. I still think this PR is useful because many repository maintainers haven't updated their packages yet.

@max06 max06 force-pushed the max06/issue181634 branch from 2b88d64 to abd9598 Compare May 8, 2023 22:53
@Tyriar Tyriar enabled auto-merge May 9, 2023 14:16
@Tyriar Tyriar merged commit cabc27e into microsoft:main May 9, 2023
@max06 max06 deleted the max06/issue181634 branch May 9, 2023 14:38
@max06
Copy link
Contributor Author

max06 commented May 9, 2023

Great experience, will commit again! 🙇🏼

@Tyriar
Copy link
Member

Tyriar commented May 9, 2023

@max06 good to hear, we're no experts in fish scripting so this was very helpful!

@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2023
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.

Regression: Shell-Integration for fish-shell broken
4 participants