Fix drag and drop on '+' button for drive letters #10842

ianjoneill · 2021-08-01T18:22:36Z

Fixes dragging and dropping drive letters onto the '+' button.

Manually tested - dragging and dropping the C:\ drive onto the '+' button works when creating a new tab, splitting or creating a new window. Dragging and dropping a regular directory still works.

Closes #10723

DHowett · 2021-08-02T17:34:24Z

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -255,10 +255,10 @@ namespace winrt::TerminalApp::implementation

            std::wstring pathText = path.wstring();

-            // Handle edge case of "C:\\", seems like the "StartingDirectory" doesn't like path which ends with '\'


I'm actually quite surprised by this premise. The reason we have issues with StartingDirectory when passed through the commandline are ones of escaping.

Does it really beef it if we don't do this trick at all? huh.

(because the commandline escaping for wt results in "C:\" being parsed as C:\" (terminating quote captured in path))

Yes - this is purely for the "Open in new window" case

If I comment out the pathText.push_back(L'.') and drop on C:\ with shift pressed, the new window fails to parse the command line:

Drag and dropping C:\ to get a new tab or split the current window works fine, it's just the new window case that fails.

OH. Okay.

We should fix this in the New Window commandline generator, honestly. That would also fix it for shift-clicking the cmd2 profile I created to test this. 😄

I suspect that this code lives in NewTerminalArgs::ToCommandline.

Done - with the added bonus that this now has a test 🙂

Love it! Thank you!

zadjii-msft

I love that you added a test, thanks!

ghost · 2021-08-03T18:14:40Z

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost · 2021-08-31T17:13:54Z

🎉Windows Terminal Preview v1.11.2421.0 has been released which incorporates this pull request.:tada:

Handy links:

Fix drag and drop on '+' for drive letters

f9d0135

ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Aug 1, 2021

DHowett reviewed Aug 2, 2021

View reviewed changes

Address review comment

909a9ac

zadjii-msft approved these changes Aug 2, 2021

View reviewed changes

DHowett approved these changes Aug 3, 2021

View reviewed changes

ghost merged commit cccaab8 into microsoft:main Aug 3, 2021

DHowett removed the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label Aug 25, 2021

ghost mentioned this pull request Aug 31, 2021

D&D on the '+' button: Dragging a drive letter (c:\) behaves weird #10723

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix drag and drop on '+' button for drive letters #10842

Fix drag and drop on '+' button for drive letters #10842

ianjoneill commented Aug 1, 2021 •

edited

DHowett Aug 2, 2021

DHowett Aug 2, 2021 •

edited

DHowett Aug 2, 2021

ianjoneill Aug 2, 2021

ianjoneill Aug 2, 2021

DHowett Aug 2, 2021

ianjoneill Aug 2, 2021

DHowett Aug 3, 2021

zadjii-msft left a comment

ghost commented Aug 3, 2021

ghost commented Aug 31, 2021

		@@ -255,10 +255,10 @@ namespace winrt::TerminalApp::implementation

		std::wstring pathText = path.wstring();

		// Handle edge case of "C:\\", seems like the "StartingDirectory" doesn't like path which ends with '\'

Fix drag and drop on '+' button for drive letters #10842

Fix drag and drop on '+' button for drive letters #10842

Conversation

ianjoneill commented Aug 1, 2021 • edited

DHowett Aug 2, 2021

Choose a reason for hiding this comment

DHowett Aug 2, 2021 • edited

Choose a reason for hiding this comment

DHowett Aug 2, 2021

Choose a reason for hiding this comment

ianjoneill Aug 2, 2021

Choose a reason for hiding this comment

ianjoneill Aug 2, 2021

Choose a reason for hiding this comment

DHowett Aug 2, 2021

Choose a reason for hiding this comment

ianjoneill Aug 2, 2021

Choose a reason for hiding this comment

DHowett Aug 3, 2021

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

ghost commented Aug 3, 2021

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 (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Aug 31, 2021

ianjoneill commented Aug 1, 2021 •

edited

DHowett Aug 2, 2021 •

edited

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 (`@msftbot`) and give me an instruction to get started! Learn more here.