Persist Runtime TabTitle and Maximize/Focus/Fullscreen states #12073

Rosefield · 2021-12-31T03:58:39Z

This commit adds additional information to the persisted window layouts.

Runtime tab titles
Focus, Maximized, and Fullscreen modes.

Also,

Adds actions for Set{Focus,FullScreen} that take a boolean
to work in addition to the current Toggle{Focus, Fullscreen} actions.
Adds SetMaximized that takes a boolean.
This adds the capability to maximize (resp restore) a window using
standard terminal actions.
This also involves hooking up a good amount of state tracking between
the terminal page and the window to see when maximize state has changed
so that it can be persisted.
These actions are not added to the default settings, but they could be.
The intention is that they could assist with automation (and was originally)
how I planned on persisting the state instead of augmenting the LaunchMode.
The fullscreen/maximized saving isn't perfect because we don't have a
way to save the non-maximized/fullscreen size, so exiting the modes
won't restore whatever the previous size was.

References #9800
Closes #11878
Closes #11426

- Runtime tab titles - Focus, Maximized, and Fullscreen modes. Also, - Adds actions for Set{Focus,FullScreen} that take a boolean to work in addition to the current Toggle{Focus, Fullscreen} actions. - Adds SetMaximized that takes a boolean. This adds the capability to maximize (resp restore) a window using standard terminal actions. This also involves hooking up a good amount of state tracking between the terminal page and the window to see when maximize state has changed so that it can be persisted. - These actions are not added to the default settings, but they could be. The intention is that they could assist with automation (and was originally) how I planned on persisting the state instead of augmenting the LaunchMode.

Rosefield · 2021-12-31T03:59:42Z

2021-12-30.22-44-51.mp4

github-actions · 2021-12-31T04:01:04Z

@check-spelling-bot Report

Unrecognized words, please review:

hibits

Previously acknowledged words that are now absent

adaa coffgroup coffgrp datetime eae emplate GENPROFILE HHmm Hostx installationpath MMdd pgorepro pgort PGU Relayout

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:Rosefield/terminal.git repository
on the feature/gh11878-11426-runtime-tabtitle-maximize-focus-states branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/1e22bfa8d6f86d0546c3f162d171274cf453db65.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/terminal/issues/comments/1003259532" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

... misspelled, then please correct them instead of using the command.
... names, please add them to .github/actions/spelling/allow/names.txt.
... APIs, you can add them to a file in .github/actions/spelling/allow/.
... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ If you see a bunch of garbage

If it relates to a ...

well-formed pattern

See if there's a pattern that would match it.

If not, try writing one and adding it to a patterns/{file}.txt.

Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

Note that patterns can't match multiline strings.

binary-ish string

Please add a file path to the excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

zadjii-msft

I'm gonna hold my s/o on the maximizedFocus thing, but otherwise this looks great, thanks!

zadjii-msft · 2022-01-05T12:52:57Z

src/cascadia/TerminalApp/TerminalPage.cpp

+        if (_isInFocusMode)
+        {
+            mode = WI_SetFlag(mode, LaunchMode::FocusMode);
+        }
+        if (_isMaximized)
+        {
+            mode = WI_SetFlag(mode, LaunchMode::MaximizedMode);
+        }


technically, the terminal could be maximized, in focus mode, (which is different than fullscreen), but this would only save it as maximized, right?

No, this would save potentially Focus | Maximized | Fullscreen if you have all 3 enabled.

derp, that's what I get for reviewing at 7am. I forgot that mode = WI_SetFlag(mode would be additive like that. Ignore me :P

zadjii-msft · 2022-01-05T12:53:58Z

src/cascadia/TerminalApp/TerminalTab.cpp

@@ -463,6 +463,17 @@ namespace winrt::TerminalApp::implementation
            state.args.emplace_back(std::move(setColorAction));
        }

+        if (!_runtimeTabText.empty())


typically I'd prefer a separate PR just for the sake of potential easy reverts, but in this case meh it's fine, this is a simple enough change

You're not wrong. I just happened to be looking to fix multiple open issues so the runtime text that took 2 minutes and the window state tracking that took an afternoon ended up in the same PR.

carlos-zamora

Mainly concerned about the side-effects of the TerminalSettingsSerializationHelpers change. I'd prefer if we could switch to a JSON_FLAG_MAPPER and update the docs/schema all in one push, but that might break the settings UI too. Ugh. Not worth doing in this PR tbh.

So I guess all I want is:

confirmation that the global setting works as expected with the new values
~~update the docs with new global setting values~~ secret feature
~~update the schema with new global setting values~~ secret feature

src/cascadia/TerminalApp/TerminalPage.cpp

carlos-zamora · 2022-01-06T16:31:39Z

src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h

@@ -219,12 +219,15 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::FirstWindowPrefe

 JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::LaunchMode)
 {
-    JSON_MAPPINGS(5) = {
+    JSON_MAPPINGS(8) = {


I'm a bit conflicted about this. At this point, why not use JSON_FLAG_MAPPER? I guess we already introduced maximizedFocus as a valid global setting option, so this would be a breaking change.

To fix this, we would have to manually handle maximizedFocus, and that just seems like a pain to do right now (with very little benefit tbh). Could you file this as a follow-up please?

Another thing, what happens if we use one of these in the "launchMode" global setting? Does it work as expected? We'll have to update the schema and docs for this too, right?

I didn't know about JSON_FLAG_MAPPER, I can look into that.

I haven't tested, but hypothetically if you change the global setting to one of the new options it should launch correctly (since it uses the same code path as restoring), although I don't think it would display in the settings UI properly. With that said, while the global setting could work with this, I didn't intend on publicizing that / extending that functionality.

Bah, the settings editor does not like the change here with more options, I'll fix that.

option one, give the user all options:

option two, let the user set any of the options in settings.json, but only show the previous options in the ui:

I'm partial to two since I really didn't intend on publicly expanding the scope of the global settings. I confirmed that the new options work if you set them in the json at least so the enterprising user would not cause the application to crash or anything.

bah, I'm ok with option 2. IIRC that means that if you hit save in the ui, the setting won't be overwritten unless you actually change/interact with the setting via the ui? (mind confirming that)

If that's the case, we should be good to go. We'll just treat the extra launchMode global setting values as a "secret feature" because the alternative is an unnecessary pain to face haha.

Yes, if you hit save without interacting with the launch mode radio buttons it will preserve whatever value you put in the json.

carlos-zamora · 2022-01-07T15:38:51Z

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

+  <data name="Globals_LaunchModeMaximizedFullscreen.Content" xml:space="preserve">
+    <value>Maximized full screen</value>
+    <comment>An option to choose from for the "launch mode" setting. Opens the app maximized and in full screen.</comment>
+  </data>
+  <data name="Globals_LaunchModeFullscreenFocus.Content" xml:space="preserve">
+    <value>Full screen focus</value>
+    <comment>An option to choose from for the "launch mode" setting. Opens the app in full screen and in focus mode.</comment>
+  </data>
+  <data name="Globals_LaunchModeMaximizedFullscreenFocus.Content" xml:space="preserve">
+    <value>Maximized full screen focus</value>
+    <comment>An option to choose from for the "launch mode" setting. Opens the app maximized in full screen and in focus mode.</comment>
+  </data>


Do we actually need these since we're removing them in Launch.cpp?

Technically this was the way that had the least amount of code. Basically the macro currently used attempts to look up every possible enum value in the localization files and so it would crash if it didn't exist. I could've written a different macro/manually write the code so it didn't try the values being filtered out. But since I added the text for the one option I was just lazy and left it in.

I appreciate this actually. Thanks.

carlos-zamora

Let's do it! Thanks!

carlos-zamora · 2022-01-10T16:10:44Z

@msftbot merge this in 10 minutes

ghost · 2022-01-10T16:10:47Z

Hello @carlos-zamora!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I won't merge this pull request until after the UTC date Mon, 10 Jan 2022 16:20:46 GMT, which is in 10 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

DHowett · 2022-01-10T16:14:46Z

src/cascadia/TerminalSettingsEditor/Launch.cpp

@@ -19,6 +19,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation

        INITIALIZE_BINDABLE_ENUM_SETTING(FirstWindowPreference, FirstWindowPreference, FirstWindowPreference, L"Globals_FirstWindowPreference", L"Content");
        INITIALIZE_BINDABLE_ENUM_SETTING(LaunchMode, LaunchMode, LaunchMode, L"Globals_LaunchMode", L"Content");
+        // More options were added to the JSON mapper when the enum was made into [Flags]
+        // but we want to preserve the previous set of options in the UI.
+        _LaunchModeList.RemoveAt(7); // maximizedFullscreenFocus


My only concern is that we should file a followup and reference it in the code -- otherwise we'll forget. We should get to this during the overhaul. Heck, the followup might even be "make the launch mode UI a collection of flags"

ghost · 2022-02-03T18:25:27Z

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

Handy links:

Rosefield added 2 commits December 30, 2021 17:56

Save the runtime tab title if it exists.

f70126b

ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Dec 31, 2021

Rosefield added 2 commits December 30, 2021 23:02

spellchecker bot didnt like my variable name

b089426

schema

d43bd9b

zadjii-msft self-assigned this Jan 4, 2022

ghost added Area-Windowing Window frame, quake mode, tearout Priority-2 A description (P2) labels Jan 5, 2022

zadjii-msft reviewed Jan 5, 2022

View reviewed changes

zadjii-msft assigned Rosefield and unassigned zadjii-msft Jan 5, 2022

zadjii-msft approved these changes Jan 5, 2022

View reviewed changes

zadjii-msft unassigned Rosefield Jan 5, 2022

zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Jan 5, 2022

ghost requested review from miniksa, carlos-zamora, DHowett, leonMSFT, PankajBhojwani and lhecker January 5, 2022 15:28

carlos-zamora requested changes Jan 6, 2022

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 6, 2022

carlos-zamora assigned Rosefield Jan 6, 2022

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 6, 2022

Code review changes, fix the settings ui

2408085

Rosefield requested a review from carlos-zamora January 6, 2022 22:33

carlos-zamora reviewed Jan 7, 2022

View reviewed changes

carlos-zamora approved these changes Jan 10, 2022

View reviewed changes

carlos-zamora unassigned Rosefield Jan 10, 2022

ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 10, 2022

DHowett removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 10, 2022

DHowett reviewed Jan 10, 2022

View reviewed changes

DHowett approved these changes Jan 10, 2022

View reviewed changes

ghost merged commit acbfb30 into microsoft:main Jan 10, 2022

Rosefield deleted the feature/gh11878-11426-runtime-tabtitle-maximize-focus-states branch January 14, 2022 19:36

This was referenced Feb 3, 2022

Terminal doesn't remember maximized, focus, fullscreen states #11426

Closed

Terminal should also restore runtime tab titles #11878

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist Runtime TabTitle and Maximize/Focus/Fullscreen states #12073

Persist Runtime TabTitle and Maximize/Focus/Fullscreen states #12073

Rosefield commented Dec 31, 2021 •

edited by DHowett

Rosefield commented Dec 31, 2021

github-actions bot commented Dec 31, 2021 •

edited

zadjii-msft left a comment

zadjii-msft Jan 5, 2022

Rosefield Jan 5, 2022

zadjii-msft Jan 5, 2022

zadjii-msft Jan 5, 2022

Rosefield Jan 5, 2022

carlos-zamora left a comment •

edited

carlos-zamora Jan 6, 2022

Rosefield Jan 6, 2022 •

edited

Rosefield Jan 6, 2022 •

edited

Rosefield Jan 6, 2022 •

edited

carlos-zamora Jan 6, 2022

Rosefield Jan 6, 2022

carlos-zamora Jan 7, 2022

Rosefield Jan 7, 2022

DHowett Jan 10, 2022

carlos-zamora left a comment

carlos-zamora commented Jan 10, 2022

ghost commented Jan 10, 2022

DHowett Jan 10, 2022

ghost commented Feb 3, 2022

Persist Runtime TabTitle and Maximize/Focus/Fullscreen states #12073

Persist Runtime TabTitle and Maximize/Focus/Fullscreen states #12073

Conversation

Rosefield commented Dec 31, 2021 • edited by DHowett

Rosefield commented Dec 31, 2021

github-actions bot commented Dec 31, 2021 • edited

@check-spelling-bot Report

Unrecognized words, please review:

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlos-zamora left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rosefield Jan 6, 2022 • edited

Choose a reason for hiding this comment

Rosefield Jan 6, 2022 • edited

Choose a reason for hiding this comment

Rosefield Jan 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlos-zamora left a comment

Choose a reason for hiding this comment

carlos-zamora commented Jan 10, 2022

ghost commented Jan 10, 2022

Choose a reason for hiding this comment

ghost commented Feb 3, 2022

Rosefield commented Dec 31, 2021 •

edited by DHowett

github-actions bot commented Dec 31, 2021 •

edited

carlos-zamora left a comment •

edited

Rosefield Jan 6, 2022 •

edited

Rosefield Jan 6, 2022 •

edited

Rosefield Jan 6, 2022 •

edited