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

Allow the user's "open pane" bindings to open a pane with the current pane #1756

Closed
zadjii-msft opened this issue Jul 1, 2019 · 24 comments · Fixed by #4683
Closed

Allow the user's "open pane" bindings to open a pane with the current pane #1756

zadjii-msft opened this issue Jul 1, 2019 · 24 comments · Fixed by #4683
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@zadjii-msft
Copy link
Member

Currently, the split pane keybindings will always open the default profile.

Perhaps a user will want to open a pane with the currently active pane's profile instead?

This is not #998, which is more about opening a pane with a specific profile, while this is about changing the default split behavior.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the 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 Jul 1, 2019
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Jul 1, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 1, 2019
@DHowett-MSFT
Copy link
Contributor

Bear with me for a second. Somebody suggested this in another issue, and I wonder if we should consider it here.

image

The new pane is focused, and put in a [number entry] or [search for profiles by name] mode, just like one of our earlier issues suggested.

We probably want that UI anyway, since it has some benefits. Thoughts?

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 2, 2019
@zadjii-msft
Copy link
Member Author

Hmmmmmmm. I don't hate the idea. That's a good idea.

Lets take it another step. With the context of #1142, we could add an arg(s) that says "open the picker dialog" or "open my default" or "open this specific profile", and default to the "open the picker" setting. Then, the user could have one keybinding for "split the default" and another for "split with the picker" (which is the settings I would use :P)

@fpintos
Copy link
Member

fpintos commented Aug 9, 2019

My first inclination is to ask the new pane to inherit everything from the parent pane - the shell, its current working directory, environment variables (maybe?), etc, the reason being that I mentally want to split the existing shell, as-is.

I can see how others might want to split the pane into something completely new, so the idea of asking which shell to use is ok as well, either thru UI or settings in the keybindings.

So, having different commands and keybindings for them seems to be the better solution.

I'll argue that there are many different cases when splitting the pane:

  • start the same shell, inheriting the environment of the parent pane (profile,working dir, environment var, etc).
  • start the same shell, but do not inherit the current environment (ie, same as starting a new tab).
  • start the default shell
  • start an arbitrary shell

@DHowett-MSFT
Copy link
Contributor

Just chiming in;

_ (profile,working dir, environment var, etc)._

Anything about the actual process on the other end is, in the general case, impossible to replicate. The connected process could be ssh.exe, whose environment variables and working directory have no bearing on the detectable environment and working directory from the terminal side. The same actually, weirdly enough, applies to WSL. It doesn't use "working directory" and it doesn't expose its environment variables to interested Windows processes in any way.

Powershell doesn't even set the current working directory, so its directory can't be detected (!) either.

@zadjii-msft
Copy link
Member Author

zadjii-msft commented Aug 9, 2019

Other related threads with thoughts on this discussion:
#2315 (comment)
#1536 (comment)

Dammit Dustin beat me

@gingters
Copy link

I really like the idea to change the default to use the current profile as default when creating a new pane with the default shortcut, and not the 'default profile'.

Using another shortcut (not the main shortcut though), should open a pane and ask for the profile to open there (#3586).

@surya-prakash-susarla
Copy link
Contributor

Just chiming in;

_ (profile,working dir, environment var, etc)._

Anything about the actual process on the other end is, in the general case, impossible to replicate. The connected process could be ssh.exe, whose environment variables and working directory have no bearing on the detectable environment and working directory from the terminal side. The same actually, weirdly enough, applies to WSL. It doesn't use "working directory" and it doesn't expose its environment variables to interested Windows processes in any way.

Powershell doesn't even set the current working directory, so its directory can't be detected (!) either.

Was interested in this but seems like it's not going to happen. Just curious if something like 'start' in cmd can't work in terminal. Is this the case ?

@zadjii-msft
Copy link
Member Author

@Surya-06 could you clarify what use case you're looking for? start is a pretty powerful command that ends up calling ShellExecute.

Are you just looking for the ability to open a new terminal in the current directory? That'll work with #4023 and wt -d . Though, opening new panes/tabs in the current Terminal window like that is much trickier.

@surya-prakash-susarla
Copy link
Contributor

@zadjii-msft I'm trying to clone the current terminal into a new pane. I need the same environment var's etc carried over similar to how 'start' does it in cmd. I know it isn't currently possible but is there a temporary workaround?

@zadjii-msft
Copy link
Member Author

I don't think there is, no. That's actually a pretty good feature request that warrants its own discussion, so I'm going to move it to it's own thread.

@tlmii
Copy link
Member

tlmii commented Feb 11, 2020

It seems like, at very least, we need an option to specify "current profile" when executing the splitPane action. I can configure different key bindings for each profile, or remember which order they are and use the index. But just having something I can specify on the splitPane action like:

    "keybindings": [
        {
            "keys": ["ctrl+|"],
            "command": {
                "action": "splitPane",
                "split": "vertical",
                "useCurrentProfile": true
            }
        }
    ]

would be good enough. I do think this should be the default ("splitting the current profile into two instances of the same profile") and that keeping the environment between them would be nice. But in the interim, just being able to split the current profile at all would be helpful.

@surya-prakash-susarla
Copy link
Contributor

surya-prakash-susarla commented Feb 15, 2020

@zadjii-msft Hey, has there been any update in this situation? I'm trying to use terminal but this problem is the main blocker preventing me from switching over. It would be great if there was a way around. Thanks !

@zadjii-msft
Copy link
Member Author

@Surya-06 Unfortunately no updates here. I know what needs to be done, and it's not terribly difficult, there's just other work that's a higher priority at the moment unfortunately.

If someone wanted to do this they'd need to:

  • add a type argument to the SplitPane action, that accepts two values: (as documented in Open panes with a specific profile #998)
    • "manual" (default): Assume that the user provided a profile parameter.
    • "duplicate": Use the currently focused pane's profile as the profile for the new pane
    • see ActionArgs.h, ActionArgs.idl, SplitPaneArgs
  • Update the logic in TerminalPage and AppActionHandlers to handle the new type parameter. We'd need to query the active tab to find the active profile if there is one, and use that, instead of the profile that CascadiaSettings::BuildSettings usually finds.
    • See TerminalPage::_HandleSplitPane, TerminalPage::_SplitPane, and CascadiaSettings::BuildSettings would probably also need some modification.
  • Add tests in KeybindingsTests.cpp

@surya-prakash-susarla
Copy link
Contributor

surya-prakash-susarla commented Feb 18, 2020

@zadjii-msft I'd like to take this up in my spare time if it's ok. 😉 I also have a question, from a top level your solution seems to find the current profile but will it carry over all the env vars also ?

@zadjii-msft
Copy link
Member Author

@Surya-06 Go for it 😄

@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Feb 18, 2020
@surya-prakash-susarla
Copy link
Contributor

@zadjii-msft cool. will work on this. will ping here if I run into any issues. Thanks!

@ghost ghost added the In-PR This issue has a related PR label Feb 21, 2020
@surya-prakash-susarla
Copy link
Contributor

surya-prakash-susarla commented Feb 21, 2020

@zadjii-msft Hello. I've started working on the project and am making the changes. I could make the changes fine but am unable to test it out since the build has been failing. I don't use visual studio so I'm editing on VS Code and building using cmd. I ran the instructions in dev help and am running into the following error:
image
I've raised a draft PR: #4683
Can you please provide some help to fix this? Thanks!

@zadjii-msft
Copy link
Member Author

I don't build the entire solution from the commandline myself very often, so I've never seen that particular error. However, I have a feeling that it's basically the same thing as #4211. I bet if you close all your browser windows, you should be able to free up enough memory to get past this step.

Once you get a successful build once, make sure to use bz or bx to avoid doing a clean rebuild on subsequent builds. This error only comes from building the pch.h file, and once it's built once, you shouldn't need to rebuild it unless you change it.

@surya-prakash-susarla
Copy link
Contributor

surya-prakash-susarla commented Feb 21, 2020

I've restarted my machine so that there are no running applications and I have 32gb of physical RAM. It has been several retries already. Is there any way to get around this? The issue only acknowledges the problem and the root cause but I was unable to find a way out.

@zadjii-msft
Copy link
Member Author

Oh well that's pretty unexpected 😕. Do you have VS installed? As much as I don't love using it either, Visual Studio sometimes might be able to provide a better error message for something like this.

@surya-prakash-susarla
Copy link
Contributor

surya-prakash-susarla commented Feb 23, 2020

@zadjii-msft Thanks for the tip. After reinstalling windows SDK and forcing the process to run at highest priority I could build it in the console.

@zadjii-msft
Copy link
Member Author

I'm glad you were able to get it figured out! I can honestly say I've never seen those errors before, so your SDK install must have been pretty messed up 😅

@AVSurfer123
Copy link

AVSurfer123 commented Mar 1, 2020

Are you just looking for the ability to open a new terminal in the current directory? That'll work with #4023 and wt -d . Though, opening new panes/tabs in the current Terminal window like that is much trickier.

@zadjii-msft Is there a way currently to have the new pane have the same working directory as the old pane? I tried doing "startingDirectory": "%CD%" for the splitPane keybinding but that failed.

@ghost ghost added In-PR This issue has a related PR and removed In-PR This issue has a related PR labels Mar 2, 2020
@keithn
Copy link

keithn commented Mar 5, 2020

Just chiming in with my thoughts as I just ran into this problem after installing powershell 7 and it's not my default shell... so all my splits end up being my default rather than ps7. I think it should split by forking the active shell as much as possible. But also have an option to change the active splits shell. Also to be able to swap the position of two splits. Also (heh..) to be able to take any shell from any tab and putting it into a split. Also number the splits and have a quick number hotkey jump to split. Also #4692 . Also pop split into its own tab.... Also save splits into a profile (remembering size), and then open a split profile which can be configured to automatically run specific programs. Then I think splits will be pretty cool, you can run semi ide like and open tabs with "split pane profiles" with active running terminal programs that are "widgets" (its nice you can resize a pane down to one line) showing timezones / server status / build status etc...

@ghost ghost closed this as completed in #4683 Mar 6, 2020
ghost pushed a commit that referenced this issue Mar 6, 2020
This change adds the ability to configure a pane to split by duplicating the current profile 

Closes #1756
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Mar 6, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants