Skip to content

Conversation

@davidmartos96
Copy link
Contributor

@davidmartos96 davidmartos96 commented Oct 25, 2024

Support Linux on EnvMixin to handle fish_user_paths being prepended to the PATH, overriding any configuration in the VSCode environment via the context.environmentVariableCollection API.

Fixes #232495

(The root problem was originally fixed for macOS on #99878 but it can also trigger when using the fish shell in other platforms)

More context: #226589 (comment)

cc @meganrogge

Tyriar
Tyriar previously requested changes Oct 28, 2024
@davidmartos96
Copy link
Contributor Author

@Tyriar I applied the fix conditionally based on the shell being used.

While further testing it, I noticed that the envMixin should be applied even on non-login shells with fish, because the variable $fish_user_paths is always set, regardless if the shell is login or not.
Otherwise, in order for the fix to work, the user needs to explicitly use the following config. And that kind of extra configuration is not necessary for the PATH prefixing to work on both bash and zsh.

{
  "terminal.integrated.profiles.linux": {
    "fish login": {
      "path": "fish",
      "args": [
        "-l"
      ]
    }
  },
  "terminal.integrated.defaultProfile.linux": "fish login"
}
@davidmartos96 davidmartos96 requested a review from Tyriar October 28, 2024 17:29
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.

@meganrogge could you review this please? I've only done a very high level review not digging into the details.

@davidmartos96 davidmartos96 changed the title Fix PATH prepending when using Fish on Linux Oct 29, 2024
Co-authored-by: Megan Rogge <megan.rogge@microsoft.com>
@meganrogge
Copy link
Contributor

meganrogge commented Oct 29, 2024

Thanks for working on this. I created this issue - we'll want to link it to this PR. #232495

@vs-code-engineering vs-code-engineering bot added this to the November 2024 milestone Oct 29, 2024
@meganrogge meganrogge enabled auto-merge (squash) October 29, 2024 17:40
@meganrogge meganrogge requested a review from Tyriar October 29, 2024 17:40
@meganrogge meganrogge merged commit 9ae3d69 into microsoft:main Oct 29, 2024
@davidmartos96 davidmartos96 deleted the fix_fish_path branch October 29, 2024 17:52
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

5 participants