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.

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

Support pwsh format for show-env export #411

Merged
merged 4 commits into from
Jan 10, 2025

Conversation

LittleBoxOfSunshine
Copy link
Contributor

@LittleBoxOfSunshine LittleBoxOfSunshine commented Dec 30, 2024

This PR demonstrates a solution for #395, with a slightly different strategy that what was discussed.

The issues outlined there:

  1. No pre-built in option for ouputing PowerShell syntax
  2. PowerShell has very different (and frankly fairly obtuse) escaping rules which is an impediment for 1. Specifically, env vars like CARGO_ENCODED_RUSTFLAGS take non-printable characters that unix shells will accept but pwsh won't.

Additionally, the shell escape crate used is escaping for cmd, not pwsh and wouldn't be compatible.

Pwsh does accept embedded unicode codes in strings though. If we encode the values this way, we don't need to worry about printable vs non-printable characters nor any of the specifics for what kinds of characters need to be escaped for what kinds of strings. This is a one-time, very fast operation. Once the emitted string is executed all the escapes are gone.

This approach wouldn't be good for a general-purpose escape function, but it's perfectly suitable for this use case.

The change enables users to execute a pwsh version of the source demo:

Invoke-Expression (cargo llvm-cov show-env --export-pwsh-prefix | Out-String)

Primary goal was to confirm this approach worked. If this is of interest, I can make any changes to the code organization, arg name, etc. The current organization is just what came to mind as I played around with the repo.

Manual test context with binary data in env:

Pasted image 20241229144731

Intermediate state example:

Pasted image 20241229145125

Final result:
Pasted image 20241229145255


Closes #395

src/cli.rs Outdated Show resolved Hide resolved
@asvrada
Copy link

asvrada commented Jan 5, 2025

Thanks for this!

Confirmed reading as Unicode doesn't work with PowerShell Version 5.1. But works after v6.0.0.0

Other than that, it looks good.

Copy link
Owner

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

Thanks!

docs/cargo-llvm-cov-show-env.txt Outdated Show resolved Hide resolved
Fix stale comment
Use noun instead of verb in arg name; rename related code accordingly
Make test check key name and initial symbols
@taiki-e taiki-e merged commit c0715a3 into taiki-e:main Jan 10, 2025
28 checks passed
@taiki-e
Copy link
Owner

taiki-e commented Jan 16, 2025

Published in 0.6.16.

@LittleBoxOfSunshine LittleBoxOfSunshine deleted the chhenk/pwsh-env-export branch January 19, 2025 17:53
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.

show-env doesn't work in PowerShell
3 participants