Skip to content

Fix direnv not using transformed values #960

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 6 commits into from
Sep 18, 2023

Conversation

JoaquinTrinanes
Copy link
Contributor

When using direnv like the docs indicated I noticed that I no longer had completions for binaries in the PATH. Turns out, direnv outputs the variables to load as a string (which is normal) and they're being loaded as-is, without undergoing the transformation specified in $env.ENV_CONVERSIONS.

The code I changed gets the subset of variables that need to undergo transformation, applies the transformation and merges them with the original values.

I'm not 100% happy with the implementation as it's not that idiomatic, but I couldn't find an easy way to conditionally update a record and returning a new record in the process. I tried the following approaches:

# Returns a table with values only in the diagonal
$direnv | items {|key, value| ... { $key: $transformed_value } }
# Returns a record that maps the index on the table to 1-item record
$direnv | items {|key, value| ... { $key: $transformed_value } } | into record
# This works! However, it's not very intuitive to read
$direnv | items {|key, value| ... { $key: $transformed_value } } | into record | flatten | into record

# a fully functioning one-liner would look like this
$direnv
    | items {|key,value| { $key: (if $key in ($env.ENV_CONVERSIONS | columns) { do ($env.ENV_CONVERSIONS | get $key | get from_string) $value } else { $value } ) } }
    | into record
    | flatten
    | into record

While at it, I also took the chance to change indentation to 4 spaces (the other files I checked used it) and simplify the default value instead of accessing the length of the record.

I'm open to any changes to this, curious to see what other way there might be to achieve this in an easy and readable way! 😁

@fdncred
Copy link
Contributor

fdncred commented Jun 26, 2023

I don't use direnv but when I saw your diagonal comment it made me think of transpose -ird. Maybe something like this would work $direnv | items {|key, value| ... { $key: $transformed_value } } | transpose -ird?

@JoaquinTrinanes
Copy link
Contributor Author

JoaquinTrinanes commented Jun 26, 2023

@fdncred THANK YOU! That worked 😁

I had to build the table differently, creating a key and value column instead of a list of records like I did before 😁 That avoids the diagonal issue altogether.

I had to add an is-empty check because [] | transpose -ird returns an empty list instead of an empty record, which is invalid input for load-env. I'll remember this trick in the future, it's not the first time I wanted to do a transformation similar to this one ✨

@fdncred
Copy link
Contributor

fdncred commented Sep 18, 2023

Thanks

@fdncred fdncred merged commit 6acb2d9 into nushell:main Sep 18, 2023
@JoaquinTrinanes JoaquinTrinanes deleted the fix-direnv-env branch September 18, 2023 17:30
@lcnbr
Copy link

lcnbr commented Oct 4, 2023

I'm getting the following error with this change:

 329 │         } | transpose -ird | load-env
     ·             ────┬────
     ·                 ╰── command doesn't support list<string> input
 330 │     }]

Any idea why?

@fdncred
Copy link
Contributor

fdncred commented Oct 4, 2023

@lcnbr I can think of 2 things that may cause this.

  1. The script is written for 0.85.0 or 0.85.1 of nushell and you're using an older version
  2. The script was written for an older version and you're using a newer version of nushell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants