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

Jump to bottom

Allow users to specify a base settings path, enabling multiple local state and multi-instance scenarios. #18696

Open
dmitrykok opened this issue Mar 17, 2025 · 4 comments · May be fixed by #18697
Open

Allow users to specify a base settings path, enabling multiple local state and multi-instance scenarios. #18696

dmitrykok opened this issue Mar 17, 2025 · 4 comments · May be fixed by #18697
Labels
In-PR This issue has a related PR Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@dmitrykok
Copy link

dmitrykok commented Mar 17, 2025 •
edited
Loading

Description of the new feature

Multi local state and settings and multi instance with --localstate command-line argument

Summary

Introduce a new command-line option -L or --localstate to allow users to specify a custom local state directory for Windows Terminal, enabling multiple local states, settings and multi-instance scenarios.

Enhancements:

  • Modify command-line argument parsing to support the new local state option
  • Update environment variable handling to use the new local state path

Proposed technical implementation details

Reviewer Guide

Introducing the ability to run multiple instances of Windows Terminal with separate settings and state by using the --localstate command-line argument. Modify parsing to support the new local state option. The changes modify the settings path retrieval logic and window class name generation to incorporate the environment variable, allowing for isolated settings for each instance.

This pull request adds support for a new command-line argument -L or --localstate to allow users to specify a custom local state directory for Windows Terminal. The implementation involves modifying command-line argument parsing, updating environment variable handling, and updating file utilities to use the new local state path.

Sequence diagram for handling the new localstate argument

sequenceDiagram
    participant User
    participant Command Line
    participant AppCommandlineArgs
    participant WindowEmperor
    participant FileUtils

    User->>Command Line: wt --localstate <path>
    Command Line->>AppCommandlineArgs: Parse arguments
    AppCommandlineArgs->>AppCommandlineArgs: Store local state path
    AppCommandlineArgs->>WindowEmperor: Launch Terminal
    WindowEmperor->>WindowEmperor: Handle Commandline Args
    WindowEmperor->>AppCommandlineArgs: Get Local State
    WindowEmperor->>FileUtils: Set ENV_WT_BASE_SETTINGS_PATH to local state path
    FileUtils->>FileUtils: Create directories at local state path
Loading

Updated class diagram for AppCommandlineArgs

classDiagram
  class AppCommandlineArgs {
    -std::wregex _commandDelimiterRegex
    -std::vector<std::wstring> _args
    -bool _hasParseError
    -bool _shouldExitEarly
    -bool _isHandoffListener
    -int _loadPersistedLayoutIdx
    -std::string _windowTarget
    -std::string _localState
    +FullResetState()
    +GetTargetWindow() string_view
    +GetLocalState() string_view
    -_getNewTerminalArgs(NewTerminalSubcommand& subcommand) NewTerminalArgs
  }
Loading

File-Level Changes

Change Details Files
Introduced a new command-line argument -L or --localstate to allow users to specify a custom local state directory for Windows Terminal.
  • Added _localState field to AppCommandlineArgs class to store the local state path.
  • Added -L,--localstate option to the command-line argument parser.
  • Implemented GetLocalState() method in AppCommandlineArgs to retrieve the local state path.
  • Added LocalState() property to the CommandlineArgs class in Remoting.idl and implemented it in Remoting.cpp.
  • Modified WindowEmperor::HandleCommandlineArgs to retrieve the settings path from the command line arguments.
  • Updated FileUtils::GetBaseSettingsPath to retrieve the settings path from the environment variables.
 src/cascadia/WindowsTerminal/WindowEmperor.cpp
src/cascadia/TerminalSettingsModel/FileUtils.cpp
src/cascadia/TerminalApp/AppCommandlineArgs.cpp
src/cascadia/TerminalApp/Remoting.cpp
src/cascadia/TerminalApp/AppCommandlineArgs.h
src/cascadia/TerminalApp/Remoting.h
src/cascadia/CascadiaPackage/Package-Dev.appxmanifest
src/cascadia/TerminalApp/Remoting.idl
src/cascadia/TerminalApp/Resources/en-US/Resources.resw

Motivation

I created Windows Terminal Layout Manager application. Which is an advanced yet user-friendly application designed to simplify and streamline the management of your Windows Terminal sessions. Effortlessly save, restore, and dynamically manage multiple terminal layouts and settings, ensuring a smooth and consistent workflow.
WTLayoutManager

Image

Image
@dmitrykok dmitrykok added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Mar 17, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot 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 Mar 17, 2025
@dmitrykok dmitrykok linked a pull request Mar 17, 2025 that will close this issue
Open
2 tasks
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Mar 17, 2025
@lhecker
Copy link
Member

lhecker commented Mar 19, 2025

Thank you! We'll need a little bit more time to look into this. This feature has been previously suggested at #6687 where the idea was to make this a CLI parameter.

@dmitrykok
Copy link
Author

dmitrykok commented Mar 31, 2025 •
edited
Loading

Thank you! We'll need a little bit more time to look into this. This feature has been previously suggested at #6687 where the idea was to make this a CLI parameter.

I also looked if it possible to add it as CLI parameter, but as I see LocalState folder needed on very early state even before command line is parsed. So decided to use ENV variable, in order to make less changes in terminal code. And my change is, to use other LocalState folder, which will hold settings, states, buffers. So you can manage different terminal layouts and instances. I already created application for that: WTLayoutManager

@lhecker
Copy link
Member

lhecker commented Mar 31, 2025

It's definitely true that doing it with a CLI argument is a lot more difficult, but that's because it requires cleaning up the architecture. The reason your change is simple is because environment variables are essentially global variables and like any global variable it easily transmits state between all parts of the application. This results in some concerns we had, like security implications that we can't foresee yet.

The ideal solution in my mind is to move this line up:

terminal/src/cascadia/WindowsTerminal/WindowEmperor.cpp

Line 352 in f34dbbf

const auto args = commandlineToArgArray(GetCommandLineW());

and to consolidate the many places that do argument parsing elsewhere into the WindowEmperor class.

However, doing so would be a huge burden for you, so the question to me personally (and perhaps the team) is how best to proceed with this PR.

@dmitrykok
Copy link
Author

It's definitely true that doing it with a CLI argument is a lot more difficult, but that's because it requires cleaning up the architecture. The reason your change is simple is because environment variables are essentially global variables and like any global variable it easily transmits state between all parts of the application. This results in some concerns we had, like security implications that we can't foresee yet.

The ideal solution in my mind is to move this line up:

terminal/src/cascadia/WindowsTerminal/WindowEmperor.cpp

Line 352 in f34dbbf

const auto args = commandlineToArgArray(GetCommandLineW());

and to consolidate the many places that do argument parsing elsewhere into the WindowEmperor class.
However, doing so would be a huge burden for you, so the question to me personally (and perhaps the team) is how best to proceed with this PR.

@lhecker, you right it was not so complicated to add --localstate command line argument and it is better this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
In-PR This issue has a related PR Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Milestone
No milestone
2 participants
@lhecker @dmitrykok