Skip to content

Allow inherited environment variables#14467

Merged
fdncred merged 1 commit intonushell:mainfrom
NotTheDr01ds:inherit-env
Nov 28, 2024
Merged

Allow inherited environment variables#14467
fdncred merged 1 commit intonushell:mainfrom
NotTheDr01ds:inherit-env

Conversation

@NotTheDr01ds
Copy link
Copy Markdown
Contributor

Description

Due to #14249 loading default_env.nu before the user's env.nu, variables that were defined there were overriding:

  • Inherited values
  • Some values that were set in the Rust code, such as the NU_LIB_PATH when set using --include-path.

This change checks to see if a variable already exists, uses its value if so, and sets the default value otherwise.

Note: ENV_CONVERSIONS is still "forced" to a default value regardless, as it needs to run reliably. There's probably not much reason to inherit it, but I'm open to the idea if there's a use-case.

User-Facing Changes

  • Before: Variables that were set in default_env.nu always overrode those that were inherited from the parent process or set internally
  • After: Inherited and internal environment variables will take priority.

Tests + Formatting

  • 🟢 toolkit fmt
  • 🟢 toolkit clippy
  • 🟢 toolkit test
  • 🟢 toolkit test stdlib

After Submitting

Will try to find a good place to mention this behavior in the Config chapter updates

Copy link
Copy Markdown
Member

@IanManske IanManske left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good.

@NotTheDr01ds NotTheDr01ds added the A:configuration Issue related to nu's configuration label Nov 28, 2024
@IanManske IanManske added the notes:fixes Include the release notes summary in the "Bug fixes" section label Nov 28, 2024
@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Nov 28, 2024

Agreed. This config stuff is getting better and better. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:configuration Issue related to nu's configuration notes:fixes Include the release notes summary in the "Bug fixes" section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants