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.

Sign up for GitHub

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

Clearly differentiate running elevated vs. drag/drop breaking #14946

Merged
merged 7 commits into from
Mar 17, 2023
Merged

Clearly differentiate running elevated vs. drag/drop breaking #14946

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99c9ab6
Clearly differentiate between running elevated and being able to drag…
zadjii-msft Mar 3, 2023
a3964a2
Ah so apparently only BODGY is allowed. If it's only a wee bit bodgy …
zadjii-msft Mar 3, 2023
b0dc3f1
Merge remote-tracking branch 'origin/main' into dev/migrie/b/13928-ca…
zadjii-msft Mar 8, 2023
6443ed1
notes from review - this really makes the code a lot more legible
zadjii-msft Mar 8, 2023
0fbb7b3
Merge remote-tracking branch 'origin/main' into dev/migrie/b/13928-ca…
zadjii-msft Mar 14, 2023
8f77f7d
sure fine whatever
zadjii-msft Mar 14, 2023
1495151
Merge remote-tracking branch 'origin/main' into dev/migrie/b/13928-ca…
zadjii-msft Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,12 @@ namespace winrt::TerminalApp::implementation
}
bool TerminalPage::CanDragDrop() const noexcept
{
static const auto canDragDrop = []() {
try
{
return Application::Current().as<TerminalApp::App>().Logic().CanDragDrop();
}
CATCH_LOG();
return true;
}();

return canDragDrop;
try
{
return Application::Current().as<TerminalApp::App>().Logic().CanDragDrop();
}
CATCH_LOG();
return true;
}

void TerminalPage::Create()
Expand Down
10 changes: 6 additions & 4 deletions src/types/utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ bool Utils::CanUwpDragDrop()
// There's a lot of wacky double negatives here so that the logic is
// basically the same as IsRunningElevated, but the end result semantically
// makes sense as "CanDragDrop".
static auto cannotDragDrop = []() {
static auto isDragDropBroken = []() {
try
{
wil::unique_handle processToken{ GetCurrentProcessToken() };
Expand All @@ -679,7 +679,7 @@ bool Utils::CanUwpDragDrop()
//
// See GH#7754, GH#11096
return false;
// They can not NOT drag drop -> they _can_ drag drop
// drag drop is _not_ broken -> they _can_ drag drop
}

// If they are running admin, they cannot drag drop.
Expand All @@ -688,11 +688,13 @@ bool Utils::CanUwpDragDrop()
catch (...)
{
LOG_CAUGHT_EXCEPTION();
return false;
// This failed? That's very peculiar indeed. Let's err on the side
// of "drag drop is broken", just in case.
return true;
}
}();

return !cannotDragDrop;
return !isDragDropBroken;
Copy link
Member

Choose a reason for hiding this comment

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

I mean, you did the hard part of writing the logic. Why not remove the double negative by...

  • isDragDropBroken --> canDragDrop
  • L681 false --> true
  • L693 true --> false
  • L697 !isDragDropBroken --> canDragDrop

(pipe the ! into the returned results, basically)

}

// See CanUwpDragDrop, GH#13928 for why this is different.
Expand Down