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

Add an action to "Open in File Explorer" #12859

Open
Tracked by #13445
zadjii-msft opened this issue Apr 8, 2022 · 15 comments · May be fixed by #18013
Open
Tracked by #13445

Add an action to "Open in File Explorer" #12859

zadjii-msft opened this issue Apr 8, 2022 · 15 comments · May be fixed by #18013
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@zadjii-msft
Copy link
Member

Kinda the opposite of the context menu. Open the CWD in file explorer. Trick here, shell needs to have been configured for path integration.

Maybe there's a way to make the action do nothing if the active Terminal hasn't set one?

Also relevant: #3337, #5916

Other thoughts: What if the user has a path selected? Check if that text is a path, that path exists, and if so, then open that path in explorer maybe?

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 8, 2022
@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Apr 8, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 8, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Apr 8, 2022
@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. good first issue This is a fix that might be easier for someone to do as a first contribution labels Apr 8, 2022
@vefatica
Copy link

vefatica commented Apr 8, 2022

It seems like this ought to be a feature of the shell. START . works in CMD, PowerShell, and TCC. In TCC I can assign it to a keystroke.

shell needs to have been configured for path integration

What does that mean? Does it refer to some mechanism that already exists?

@zadjii-msft
Copy link
Member Author

I mean, theoretically, yea. This is super doable in the shell. I'm more tracking this as a sub-task of the right-click context menu, or as a trigger. We need an action internally to hook up the context menu entry.

What does that mean? Does it refer to some mechanism that already exists

Yea I'm talking about the OSC9;9 shell integration, as described in https://docs.microsoft.com/en-us/windows/terminal/tutorials/new-tab-same-directory

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Apr 8, 2022
@vefatica
Copy link

vefatica commented Apr 9, 2022

Yea I'm talking about the OSC9;9 shell integration

That's pretty neat and works as advertized (along with Ctrl+Shift+d). It's the sort of thing I don't often want to do. Most likely, by the next time I want to duplicate a tab, I'll have forgotten about the keystroke (and this thread).

Are there other benefits that could come from a shell using OSC9;9 to send a string to WT?

Is OSC9;9 documented anywhere other then the tutorial you mentioned? I didn't find it in https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences.

@zadjii-msft
Copy link
Member Author

Are there other benefits that could come from a shell using OSC9;9 to send a string to WT?

At the moment not really, but who knows what we might do with it in the future. Like, maybe #12863.

OSC9;9 was originally authored by ConEmu so they've got slightly better docs https://conemu.github.io/en/ShellWorkDir.html

@vefatica
Copy link

Like, maybe #12863

That I already have in TCC (popup history and dirhistory). New TCCs use GUI windows for this by default and that doesn't work very well with WT because TCC has no clue where the command line interface window IS. So I use the older option to create the popups in the console screen buffer.

I'm aware of another thread that deals with console apps creating windows. I'm in favor of any improvements on that front, in particular, getting such "popup" windows to have a position which is based on the position of WT's tab. As it is, can WT convey such information to conhost and, if so, can a console app get it?

@bfahrenfort
Copy link

bfahrenfort commented Apr 23, 2022

in particular, getting such "popup" windows to have a position which is based on the position of WT's tab

I'm interested in this as well. We can get the WT position with GetWindowRect (winuser.h) on TerminalPage::_hostingHwnd, then it's just a question of starting the explorer window in a secure way.

Any suggestions? I'm considering CreateProcess from processthreadsapi.h.

Here's what a potential handler in AppActionHandlers could look like (albeit untested), I used the current implementation of #8330 's code for retrieving the CWD:

void TerminalPage::_HandleOpenInFileExplorer(const IInspectable& /* sender */,
                                             const ActionEventArgs& actionArgs)
{
    const auto focusedTab{ _GetFocusedTabImpl() };
    if (focusedTab)
    {
        const auto workingDirectory = focusedTab->GetActiveTerminalControl().WorkingDirectory();
        const auto validWorkingDirectory = !workingDirectory.empty();
        if (validWorkingDirectory && _hostingHwnd.has_value())
        {
            LPRECT lpRect;
            GetWindowRect(TerminalPage::_hostingHwnd.value(), lpRect);

            // Spawn Explorer
        }
    }
}

Quick update: spawning Explorer with ShellExecuteW is trivial. However, obtaining the HWND to call MoveWindow is not. I'm open to suggestions on how to proceed, I really don't want to do the established solution of "wait for new process to idle -> enumerate windows -> get window by pid" because that little stutter (which could be a big stutter on weaker systems) followed by an enumeration drops performance shockingly quickly.

I'm also lost on trying to add my handler to the Command Palette. It's registered as an action in the X-macros and I've been testing it through a Command in defaults-universal.json as well as a tab flyout context menu item which looks quite nifty:
image
However, I do think it needs to be in Command Palette for ease of use. Could someone please point me in the right direction? Thanks!

@zadjii-msft
Copy link
Member Author

However, obtaining the HWND to call MoveWindow is not

Wait hold up why do we need to do that at all? I think it's perfectly fine to just ShellExecute({the path}) and let explorer just do it's thing. At least for a first version of the feature, yea?

a Command in defaults-universal.json

Whoops, we should probably just delete that file - it's not actually used anymore 😅 defaults.json is the file you're looking for, that one should have many, MANY more actions.

@bfahrenfort
Copy link

we should probably just delete that file - it's not actually used anymore 😅 defaults.json is the file you're looking for

Hmm. I definitely put it in defaults.json as well. Here's what I used:

{ "command": "openInFileExplorer", "keys": "ctrl+shift+alt+e" },

The keyboard shortcut works, but it still doesn't show up in command palette. Do I have to add it to another place?

Side note, someone should really make a tutorial for registering an action to command palette. I'll open an issue for it when I finish figuring this one out. Thanks!

@zadjii-msft
Copy link
Member Author

Do I have to add it to another place?

Ah, probably. There's a pile of places you need to add this stuff, though, fewer than there used to be. I bet you're missing an entry in ActionAndArgs::GenerateName. That's what generates the names for actions in the command palette.

Side note, someone should really make a tutorial for registering an action to command palette

You know, there actually is a guide, but it's pretty out of date: https://github.com/microsoft/terminal/blob/main/doc/cascadia/AddASetting.md#adding-an-action. We've since added some x-macros to the project that take care of a LOT of the boilerplate, but clearly not all of it.

I might have you also refer to #12097 - looks like that was the most recent merged PR to add an action. That one also needed action args, which I don't think we need in this case, so that should simplify things a bit.

@Lee-Carre
Copy link

Lee-Carre commented Aug 25, 2023

If I'm understanding the OP correctly, then the following command will open a file manager at the current CLI working directory:

explorer .

that is; launch explorer with the first argument matching (expanded to) the current path (. = current directory, compare .. = parent directory).

Been using this for years (decades?), to great effect.

I'm not sure what the equivalent for PowerShell syntax is.

@i-zanis
Copy link

i-zanis commented Oct 22, 2023

There's an actual shortcut which is ii (invoke-item).

@KieranDevvs
Copy link

KieranDevvs commented Aug 16, 2024

What about instead of having a hidden away menu button, we detect file paths and turning them into hyperlinks?
image
image
image

It would then also work with not just the current working directory, but any path from any output.
Lets say you run a compiler and it spits out a log containing multiple paths, it would be handy to be able to ctrl+click on them to go right to the URI:

Microsoft Windows [Version 10.0.19045.4717]
(c) Microsoft Corporation. All rights reserved.

C:\Users\Kieran Devlin> compiler.exe run
Compiling 28 source files...
Binary generated in C:\Users\Kieran Devlin\sources\app1\dist\bin\my-bin.exe
Full output log: C:\Users\Kieran Devlin\sources\app1\logs\compiler\2024-08-16 12-01-56.txt

@michaeljsXu
Copy link
Contributor

I made a commit 07ab3d9 to add this as an action, but I don't see it in the command palette. It works when I attach it to the right click fly out menu though. What am I missing?

@lhecker
Copy link
Member

lhecker commented Oct 8, 2024

I believe it could be because you're missing an "id" in the defaults.json. DecreaseFontSize for instance doesn't really have anything else on top of your changes.

@michaeljsXu michaeljsXu linked a pull request Oct 8, 2024 that will close this issue
4 tasks
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Oct 8, 2024
@michaeljsXu
Copy link
Contributor

michaeljsXu commented Oct 8, 2024

Thank you that fixed the problem. I made a PR.

I saw in #13526 that the team believes actions related to CWD don't have too much value at the moment since most people don't path integration set up. Is there value in working on an action to open a highlighted directory instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants