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

Restore support for pasting files #16634

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jan 31, 2024

TIL: You could Ctrl+V files into Windows Terminal and here I am,
always opening the context menu and selecting "Copy as path"... smh

This restores the support by adding a very rudimentary HDROP handler.
The flip side of the regression is that I learned about this and so
conhost also gets this now, because why not!

Closes #16627

Validation Steps Performed

  • Single files can be pasted in WT and conhost ✅

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Priority-2 A description (P2) Product-Terminal The new Windows Terminal. labels Jan 31, 2024
@lhecker lhecker force-pushed the dev/lhecker/16627-clipboard-regression branch from 6c058a1 to c2bf907 Compare January 31, 2024 17:26
@DHowett
Copy link
Member

DHowett commented Jan 31, 2024

Hey, the WinRT API was doing something for us that we didn't have to worry too much about!

@lhecker
Copy link
Member Author

lhecker commented Jan 31, 2024

Ackchyually, we had to do it manually:

// Windows Explorer's "Copy address" menu item stores a StorageItem in the clipboard, and no text.
else if (data.Contains(StandardDataFormats::StorageItems()))
{
auto items = co_await data.GetStorageItemsAsync();
if (items.Size() > 0)
{
auto item = items.GetAt(0);
text = item.Path();
}
}

That's effectively the HDROP handler but with WinRT APIs. I removed it because I never realized that you could simply Ctrl+V a file into WT. The author of that code might not have known either because look at the comment on top of it: It only refers to the whacky "Copy address" menu item, and that one happens to actually contain a CF_UNICODETEXT and that's why I didn't port the HDROP handling into the Win32 code.

return {};
}

auto buffer = winrt::impl::hstring_builder{ cap };
Copy link
Member

Choose a reason for hiding this comment

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

we shouldn't use winrt::impl types directly. However, I don't love any of the other options. heh

@DHowett
Copy link
Member

DHowett commented Jan 31, 2024

I removed it because I never realized that you could simply Ctrl+V a file into WT. The author of that code might not have known either because look at the comment on top of it: It only refers to the whacky "Copy address" menu item, and that one happens to actually contain a CF_UNICODETEXT and that's why I didn't port the HDROP handling into the Win32 code.

Ah, I see.

@lhecker lhecker merged commit ef96e22 into main Feb 1, 2024
20 checks passed
@lhecker lhecker deleted the dev/lhecker/16627-clipboard-regression branch February 1, 2024 23:49
DHowett pushed a commit that referenced this pull request Feb 1, 2024
TIL: You could Ctrl+V files into Windows Terminal and here I am,
always opening the context menu and selecting "Copy as path"... smh

This restores the support by adding a very rudimentary HDROP handler.
The flip side of the regression is that I learned about this and so
conhost also gets this now, because why not!

Closes #16627

* Single files can be pasted in WT and conhost ✅

(cherry picked from commit ef96e22)
Service-Card-Id: 91727725
Service-Version: 1.19
DHowett pushed a commit that referenced this pull request Feb 21, 2024
TIL: You could Ctrl+V files into Windows Terminal and here I am,
always opening the context menu and selecting "Copy as path"... smh

This restores the support by adding a very rudimentary HDROP handler.
The flip side of the regression is that I learned about this and so
conhost also gets this now, because why not!

Closes #16627

## Validation Steps Performed
* Single files can be pasted in WT and conhost ✅

(cherry picked from commit ef96e22)
Service-Card-Id: 91727726
Service-Version: 1.20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal.
Projects
Development

Successfully merging this pull request may close these issues.

[1.19] Pasting a file from explorer into the Terminal no longer pastes the path (regression from 1.18)
3 participants