Refactor the SettingsTab to be a pane

[zadjii-msft]

Summary of the Pull Request

... technically. We still won't let it actually be a pane, but now it acts like one. It's hosted in a SettingsPaneContent. There's no more SettingsTab. It totally can be in a pane (but don't?)

targets: #16171

References and Relevant Issues

• #997

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

• Still opens the settings
• Only opens a single settings tab, or re-activates the existing one
• Session restores!
• Updates the title of the tab appropriately
• I previously did use the scratchpad action to open the settings in a pane, and that worked.

other PRs

• #16170
• #16171
• #16172 <-- you are here

[github-actions[bot]]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (1)

somehown

To accept :heavy_check_mark: these unrecognized words as correct, run the following commands

... in a clone of the [email protected]:microsoft/terminal.git repository on the dev/migrie/f/sui-panes branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/6512547745/attempts/1'

Pattern suggestions :scissors: (1)

You could add these patterns to .github/actions/spelling/patterns/fb74fc8c6a5f1377f4f8b95618a667cbd0742d29.txt:

# Automatically suggested patterns
# hit-count: 1 file-count: 1
# tput arguments -- https://man7.org/linux/man-pages/man5/terminfo.5.html -- technically they can be more than 5 chars long...
\btput\s+(?:(?:-[SV]|-T\s*\w+)\s+)*\w{3,5}\b

Warnings (1)

See the :open_file_folder: files view or the :scroll:action log for details.

:information_source: Warnings	Count
:information_source: candidate-pattern	1

See :information_source: Event descriptions for more information.

:pencil2: 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.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

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.

:microscope: 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. :wink:

If the flagged items are :exploding_head: false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

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

• well-formed pattern.

  If you can write a pattern that would match it, try adding it to the patterns.txt file.

  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.

[zadjii-msft]

@carlos-zamora Yea honestly, the whole TerminalSettings update is wacky. Honestly, I'm thinking of something even weirder:

• TerminalPage should just UpdateSettings all it's tabs
• All the tabs should just UpdateSettings all their panes
• CascadiaSettings itself stashes a std::unordered_map<winrt::guid, std::pair<Profile, TerminalSettingsCreateResult>> profileGuidSettingsMap.
  • Clear that on settings reload
• Inside TerminalPaneContent::UpdateSettings, we do a settings->LookupTerminalSettingsForUpdate to get the TerminalSettings
  • then, CascadiaSettings would own the TerrminalSettings caching, which seems like a good idea to me

[DHowett]

Despite living in the same library, TerminalSettings and CascadiaSettings live in fundamentally different "layers" of the application. TerminalSettings is only where it is because it is used by the settings editor and the app itself, but it it an app affine object. CascadiaSettings should not have any more knowledge of it than it already does. :)

[zadjii-msft]

Uuuuhhhhhggggg I forgot about that. Passing in the profile/TerminalSettings cache as a param to TerminalTab&Pane's UpdateSettings doesn't seem like the worst thing in the world. Still doesn't feel great though

[DHowett]

Does this let you get rid of the wacky inheritance tree from Tab/TabBase/TerminalTab/SettingsTab? If so, will you file followup work (and do it?)

[zadjii-msft]

Does this let you get rid of the wacky inheritance tree

I mean the tree is now

• TabBase
  • TerminalTab

but yes, you know what? I'll do that this week and stack it on here

[zadjii-msft]

but yes, you know what? I'll do that this week and stack it on here

This is in https://github.com/microsoft/terminal/compare/dev/migrie/f/sui-panes...dev/migrie/b/remove-terminaltab

That's ready to go here, but there's also still another two PRs stacked on top of here. Let's merge those in too, before the noisy rename & move