-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Replace the UWP file export with the shell32 one #11365
Conversation
This comment has been minimized.
This comment has been minimized.
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 think the only thing that feels weird to me is that we export a file using CascadiaSettings::ExportFile()
. I get that we're doing it so that we can leverage FileUtils.h
, but would it be better for us to move FileUtils
into the inc
folder and just have it shared between projects? The only thing I think we would have to move out would be GetBaseSettingsPath
.
That said, that's probably better as a CodeHealth follow-up, so I don't mind this for now.
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
} | ||
CATCH_LOG(); // non-fatal | ||
THROW_IF_FAILED(dialog->SetFileTypes(ARRAYSIZE(supportedFileTypes), supportedFileTypes)); |
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.
std::size
is the modern ARRAYSIZE
.
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.
huh, to think i've been using std::extent<decltype(x)>::value
all this time. thanks!
Just like in #9760, we can't actually use the UWP file picker API, because it will absolutely not work at all when the Terminal is running elevated. That would prevent the picker from appearing at all. So instead, we'll just use the shell32 one manually.
This also gets rid of the confirmation dialog, since the team felt we didn't really need that. We could maybe replace it with a Toast (#8592), but meh