search

microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.77k stars 8.34k forks source link

TerminalPaneContent.cpp - TerminalPaneContent::TerminalPaneContent never knows the profile settings updated with the wt.exe cmdline arguments #17473

Open htcfreek opened 4 months ago

htcfreek commented 4 months ago

Windows Terminal version

16497 (which is in sync with main) - PR-Commit: 756d7b3e7860d1082ed87833521ed65638688abc

Windows build number

10.0.19045.4529

Other Software

No response

Steps to reproduce

Try to access the profile configuration used by the current terminal pane content in a version that is updated based on the command line parameter used to start wt.exe .

Expected Behavior

The _profile variable contains the current profile settings from settings file. And the _cache contains the settings updated based on the wt.exe command line arguments.

Actual Behavior

The _profile variable contains the current profile settings from settings file. And the _cache contains the current profile settings from settings file too.

Debug information

You can use my PR #16497 and check the value of const auto closeMode in TerminalPaneContent::_controlConnectionStateChangedHandler. It is always the one from settings and the same as the one from _profile.

If you compare the value against the value returned in AppCommandlineArgs::_getNewTerminalArgs you will see that the command line parsing works.

DHowett commented 4 months ago

This is because it only checks the profile, and there is no feature that override the profile's value for CloseOnExit here. I would somewhat expect a PR that adds --keepOpen support to plumb through overriding the CloseOnExit value in an appropriate way.

Expected Behavior

And the _cache contains the settings updated based on the wt.exe command line arguments.

FWIW: _cache is supposed to be a cache; it is NOT supposed to store novel and important values.

htcfreek commented 4 months ago

I did the exact same as done for other commandline arguments.

Today I tried to implement a new profile property (that is not a setting) for holding the override value. But I failed because I wasn't able to access it in the command line parser logic. (The point is that implementing a setting makes no sense to me as it would be written to/read from json.)

htcfreek commented 4 months ago

I would somewhat expect a PR that adds --keepOpen support to plumb through overriding the CloseOnExit value in an appropriate way.

@DHowett Wher in the code would you override it? The place wher I override it (TerminalSettings::CreateWithNewTerminalArgs()) doesn't work as I can't acces the overridden value in TerminalPaneContent::_controlConnectionStateChangedHandler().

htcfreek commented 5 days ago

@zadjii-msft I know you have many things to do. But it would be great if you can take a look. (I have not enough c++ knowledge to find the cause of my problem.)