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

Manually pre-evaluate the starting directory when calling elevate-shim #15286

Merged

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented May 4, 2023

targets #15280

When ctrl+clicking on a profile, pre-evaluate the starting directory of that profile, and stash that in the commandline we pass to elevate shim.

So in the case of something like "use parent process directory", we'll run elevate-shim new-tab -p {guid} -d "C:\\the path\\of\\terminal\\."

Closes #15173

  In the dirtiest way possible, this seems to work for most "start with this CWD" scenarios.
@zadjii-msft

This comment was marked as resolved.

@zadjii-msft
Copy link
Member Author

230be3e fixed the earlier issue I saw, where a profile that wasn't a relative path would actually just open in the vcwd. Now this branch works too.

@zadjii-msft zadjii-msft marked this pull request as ready for review May 4, 2023 16:35
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels May 12, 2023
Base automatically changed from dev/migrie/b/5506-virtual-cwd to main May 12, 2023 18:20
@github-actions

This comment has been minimized.

return winrt::hstring{
Utils::EvaluateStartingDirectory(cwdString, std::wstring_view{ path })
};
return Utils::EvaluateStartingDirectory(_WindowProperties.VirtualWorkingDirectory(), path);
}
Copy link
Member

Choose a reason for hiding this comment

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

winrt::hstring is implicitly convertible to std::wstring_view, so there's no need for explicit casts like this most of the time. It just doesn't work for something like std::filesystem::path::operator/=, because C++ wouldn't know which of the 31 overloads to call, given a hstring.

Also, when cppwinrt generates String parameters for functions, it'll not use winrt::hstring but winrt::param::hstring which binds to almost any string type, include std::wstring, without making any extra copies on the heap. We can thus just return a std::wstring from this function and it'll work without any additional modifications.

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 15, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/migrie/b/15173-ctrl-click-same-cwd branch May 15, 2023 20:10
DHowett pushed a commit that referenced this pull request May 15, 2023
#15286)

_targets #15280_

When ctrl+clicking on a profile, pre-evaluate the starting directory of
that profile, and stash that in the commandline we pass to elevate shim.

So in the case of something like "use parent process directory", we'll
run `elevate-shim new-tab -p {guid} -d "C:\\the path\\of\\terminal\\."`

Closes #15173

---------

Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
(cherry picked from commit 9a4f4ab)
Service-Card-Id: 89180220
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Commandline wt.exe's commandline arguments AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. zBugBash-Consider
Projects
Development

Successfully merging this pull request may close these issues.

After updating Windows Terminal, pressing Ctrl+Profile will not inherit the working directory
3 participants