-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Create til::env to help with refreshing environment variables #12516
Conversation
This comment has been minimized.
This comment has been minimized.
@lhecker this can be yours now if you want it. |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view or the 📜action log for details. Unrecognized words (8)autoexec Previously acknowledged words that are now absentCLA demoable everytime Hirots inthread reingest unmark :arrow_right:To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands... in a clone of the git@github.com:microsoft/terminal.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/3996360350/attempts/1' ✏️ Contributor please read thisBy default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
See the 🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉 If the flagged items are 🤯 false positivesIf items relate to a ...
|
I've added a commit to this on my fork of the terminal that no longer extends from |
|
||
std::wstring expand_environment_strings(std::wstring input) | ||
{ | ||
// TODO: this should be replacing from ourselves, not from the OS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take it this method needs implementing as follows - or is this too naive?
Iterate through the string - append characters to a "return" buffer until you find a percent character. At this point start appending the characters to a "key" buffer instead. When you find a "closing" percent, look the key up in the map. If found, append the looked up value to the return buffer, otherwise append the key as was (including the percent characters), then reset the state. If at the end there's stuff left in the key buffer, append it to the return buffer (including a literal percent character at the beginning).
Test cases:
- "Foo%ENV%Baz" with an environment variable %ENV% defined as "Bar" would result in the string "FooBarBaz".
- "Foo%ENV%Baz" with no %ENV% variable would result in the string "Foo%ENV%Baz".
- "Foo%ENV" would result in the string "Foo%ENV".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "Foo%Baz%ENV%" with an %ENV% variable but no %Baz% variable? CMD seems to expand %ENV% in that situation, but I don't know what ExpandEnvironmentStringsW does.
Honestly, I'd recommend just filing a new one, I think that'd be the easiest way to iterate and get this in. Thanks! |
Superseded by @ianjoneill's (excellent) work. Thanks Michael! |
Summary of the Pull Request
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed