Initialize Terminal Preview settings from Terminal settings #12907

huiyooumich · 2022-04-14T17:48:00Z

Windows Terminal Preview gets existing settings from Release build if
Preview settings are empty

This ensures that when settings are empty or not existent, we check if
we're currently in a preview build and if we are, we attempt to grab
settings from the Release build's setting path instead. We tested it
manually by changing settings in Release build and confirming that
changes migrated to Preview when settings are empty or not existent.
Additionally, we tested that settings.json of the running build changed.
We also ran existing TAEF testing locally and it passed.

In LoadAll() function in
src\cascadia\TerminalSettingsModel\CascadiaSettingsSerialization.cpp, we
first checked if the settings file us empty/exists via settingsString.
If it does not and we are in the Preview build, we try loading the
Release build's settings. We created modified versions of
CascadiaSettings::_settingsPath() and GetBaseSettingsPath() to get the
path for the Release build's settings. If the Release build settings do
exist and firstTimeSetup is true, we set it to settingsString so it can
be written to disk via WriteSettingsToDisk(). Note that currently we
hardcode the path of the Release build. This pull request was worked on
with @Dannihu01.

Validation Steps Performed Test1: Setting to firstTimeSetup is true

and loading settings.json from WT release when release exists -> Result:
settings.json AND GUI reflected WT release’s settings

Test2: Setting to firstTimeSetup is true and loading settings.json from
WT release when release doesn’t exist -> Result: settings.json AND GUI
reflected DEFAULT settings

Test3: (After running Test1) Setting to firstTimeSetup is false and
seeing if current settings.json matches WT release. (See if it doesn’t
change) -> Result: settings.json AND GUI reflected WT release’s settings

Closes #6855

Co-authored-by: Danniell Hu [email protected]

Preview gets existing settings from Release build

Utilized Invoke-CodeFormat from Code Health Script advice

zadjii-msft

Okay, I think this is pretty straightforward. I'd ✅, but I'm a little curious about how this works unpackaged, esp with Preview branding, so I want a second opinion.

zadjii-msft · 2022-04-27T20:27:06Z

src/cascadia/TerminalSettingsModel/FileUtils.cpp

+            parentDirectoryForSettingsFile /= ReleaseSettingsFolder;
+
+            if (!IsPackaged())
+            {
+                parentDirectoryForSettingsFile /= UnpackagedSettingsFolderName;
+            }


I'm not sure this is right for unpackaged versions of the Terminal. I might want to tap in @DHowett for this bit, hes a bit more familiar with running the Terminal unpackaged.

Just looking at my own files, I've only got a %localAppData%\Microsoft\Windows Terminal.

Heck, I'm not sure this is ever relevant for Preview builds running unpackaged - I think they always just use %localAppData%\Microsoft\Windows Terminal anyways. I might be wrong here.

Yeah -- preview builds just use the same storage as stable when they run unpackaged, so there's no worry here :)

zadjii-msft · 2022-04-27T20:29:00Z

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

+    bool releaseSettingExists = false;
+    if (firstTimeSetup)
+    {
+#if defined(WT_BRANDING_PREVIEW)


Clever, simple, easy to parse what's happening. 👍

zadjii-msft · 2022-04-27T20:30:16Z

(sorry for the delays in reviewing 😅)

lhecker

I believe this change is fine. With unpackaged WT, I don't believe we make a distinction for the settings path between Preview and non-Preview.

zadjii-msft · 2022-05-16T18:18:34Z

So, for unpackaged "preview" branded builds, it'll just happen to work anyways? Like, this is never an issue, unpackaged terminal just always works this way. Even if you ran an unpackaged Preview build first, with no existing settings, it would never find the "release" settings, because it would look in the nonsense path %localAppData%\Packages\Microsoft.WindowsTerminal_8wekyb3d8bbwe\LocalState\Microsoft\Windows Terminal for the settings file, definitely not find it, and continue on its merry way?

Okay I suppose I'm fine with this, but I might follow up with a cleanup PR with comments to that effect.

zadjii-msft

as noted. Thanks!

carlos-zamora · 2022-06-01T00:25:08Z

@msftbot make sure @DHowett signs off on this

ghost · 2022-06-01T00:25:13Z

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'll only merge this pull request if it's approved by @DHowett

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".

zadjii-msft · 2022-06-10T17:51:48Z

Hey @DHowett review this so this can be in 1.15.

DHowett · 2022-06-10T17:56:45Z

src/cascadia/TerminalSettingsModel/FileUtils.cpp

+            parentDirectoryForSettingsFile /= ReleaseSettingsFolder;
+
+            if (!IsPackaged())
+            {
+                parentDirectoryForSettingsFile /= UnpackagedSettingsFolderName;
+            }


Yeah -- preview builds just use the same storage as stable when they run unpackaged, so there's no worry here :)

src/cascadia/TerminalSettingsModel/FileUtils.cpp

DHowett

Thank you for doing this! Sorry I let it sit so long!

ghost · 2022-07-06T18:09:30Z

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

Handy links:

Fulya2133

function (request, socket, head) { }

Added initial migrate of Release settings to Preview

0694a4e

Preview gets existing settings from Release build

ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Apr 14, 2022

Fixed style check

022cd58

Utilized Invoke-CodeFormat from Code Health Script advice

huiyooumich changed the title ~~Added initial migrate of Release settings to Preview~~ Add migrate of Release settings to Preview Apr 14, 2022

huiyooumich changed the title ~~Add migrate of Release settings to Preview~~ Initialize Terminal Preview settings from Terminal settings Apr 14, 2022

huiyooumich marked this pull request as ready for review April 15, 2022 07:44

zadjii-msft reviewed Apr 27, 2022

View reviewed changes

zadjii-msft assigned DHowett Apr 27, 2022

lhecker approved these changes May 5, 2022

View reviewed changes

Merge branch 'main' into dev/huiyooumich/6855-intialize-preview-settings

cd7cf1e

zadjii-msft approved these changes May 16, 2022

View reviewed changes

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

zadjii-msft added this to the Terminal v1.15 milestone Jun 10, 2022

DHowett reviewed Jun 10, 2022

View reviewed changes

Update src/cascadia/TerminalSettingsModel/FileUtils.cpp

e75bc55

DHowett approved these changes Jun 10, 2022

View reviewed changes

ghost merged commit cac52a9 into microsoft:main Jun 10, 2022

ghost mentioned this pull request Jul 6, 2022

[discussion] Initialize Terminal Preview settings from Terminal settings #6855

Closed

Fulya2133 approved these changes Aug 15, 2022

View reviewed changes

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialize Terminal Preview settings from Terminal settings #12907

Initialize Terminal Preview settings from Terminal settings #12907

huiyooumich commented Apr 14, 2022 •

edited by DHowett

Loading

zadjii-msft left a comment

zadjii-msft Apr 27, 2022

DHowett Jun 10, 2022

zadjii-msft Apr 27, 2022

zadjii-msft commented Apr 27, 2022

lhecker left a comment

zadjii-msft commented May 16, 2022

zadjii-msft left a comment

carlos-zamora commented Jun 1, 2022

ghost commented Jun 1, 2022

zadjii-msft commented Jun 10, 2022

DHowett Jun 10, 2022

DHowett left a comment

ghost commented Jul 6, 2022

Fulya2133 left a comment

Initialize Terminal Preview settings from Terminal settings #12907

Initialize Terminal Preview settings from Terminal settings #12907

Conversation

huiyooumich commented Apr 14, 2022 • edited by DHowett Loading

Validation Steps Performed Test1: Setting to firstTimeSetup is true

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Apr 27, 2022

Choose a reason for hiding this comment

DHowett Jun 10, 2022

Choose a reason for hiding this comment

zadjii-msft Apr 27, 2022

Choose a reason for hiding this comment

zadjii-msft commented Apr 27, 2022

lhecker left a comment

Choose a reason for hiding this comment

zadjii-msft commented May 16, 2022

zadjii-msft left a comment

Choose a reason for hiding this comment

carlos-zamora commented Jun 1, 2022

ghost commented Jun 1, 2022

zadjii-msft commented Jun 10, 2022

DHowett Jun 10, 2022

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Jul 6, 2022

Fulya2133 left a comment

Choose a reason for hiding this comment

huiyooumich commented Apr 14, 2022 •

edited by DHowett

Loading