Skip to content

Add Elevate flag to the SUI#12257

Merged
1 commit merged intomainfrom
dev/migrie/f/elevate-in-sui
Jan 27, 2022
Merged

Add Elevate flag to the SUI#12257
1 commit merged intomainfrom
dev/migrie/f/elevate-in-sui

Conversation

@zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jan 26, 2022

Does what it says on the box

image

  • I work here
  • Discussed in OneNote

@zadjii-msft
Add Elevate flag to the SUI
71b2d29 
Does what it says on the box

{{img}}

* [x] I work here
* [x] Discussed in OneNote
* [ ] Still need to write some docs
@zadjii-msft zadjii-msft added Product-Terminal The new Windows Terminal. Area-SettingsUI Anything specific to the SUI labels Jan 26, 2022
@zadjii-msft
Copy link
Member Author

zadjii-msft commented Jan 26, 2022

May conflict with the other SUI PRs. Go do those first you barbarians!

#11720 and #12144

@ianjoneill
Copy link
Contributor

ianjoneill commented Jan 26, 2022

A related question now that this is included in the SUI:

Is there any reason why the elevate flag isn't duplicated when a profile is duplicated? (I thought maybe it wasn't included previously as it wasn't in the SUI)

If it should be, I think it's a one-line fix, adding it around here, which I can do if you like 🙂

terminal/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

Lines 283 to 297 in fe52368

DUPLICATE_SETTING_MACRO(Icon);
DUPLICATE_SETTING_MACRO(CloseOnExit);
DUPLICATE_SETTING_MACRO(TabTitle);
DUPLICATE_SETTING_MACRO(TabColor);
DUPLICATE_SETTING_MACRO(SuppressApplicationTitle);
DUPLICATE_SETTING_MACRO(UseAcrylic);
DUPLICATE_SETTING_MACRO(ScrollState);
DUPLICATE_SETTING_MACRO(Padding);
DUPLICATE_SETTING_MACRO(Commandline);
DUPLICATE_SETTING_MACRO(StartingDirectory);
DUPLICATE_SETTING_MACRO(AntialiasingMode);
DUPLICATE_SETTING_MACRO(HistorySize);
DUPLICATE_SETTING_MACRO(SnapOnInput);
DUPLICATE_SETTING_MACRO(AltGrAliasing);
DUPLICATE_SETTING_MACRO(BellStyle);

@zadjii-msft
Copy link
Member Author

zadjii-msft commented Jan 26, 2022

@ianjoneill yes please submit a PR for that. That was an oversight, I believe that the original elevate PR even predates duplicating profiles by a good amount 😅

@DHowett
Copy link
Member

DHowett commented Jan 26, 2022

@ianjoneill yes please submit a PR for that. That was an oversight, I believe that the original elevate PR even predates duplicating profiles by a good amount 😅

How did the X macros for settings not help this?

@zadjii-msft
Copy link
Member Author

zadjii-msft commented Jan 26, 2022 •
edited
Loading

How did the X macros for settings not help this?

Apparently we didnt use the macro for CascadiaSettings::DuplicateProfile!

UseAtlasEngine, BellSound, and Elevate are all missing from the duplicate. Connection type too, but that one's... tricky maybe. Opacity is duplicated, but it's not otherwise in the appearance macro. Uh oh.

@zadjii-msft zadjii-msft mentioned this pull request Jan 26, 2022
Closed
@zadjii-msft zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Jan 27, 2022
@DHowett
Copy link
Member

DHowett commented Jan 27, 2022

@PankajBhojwani make sure that you get this one merged in!

@DHowett
Copy link
Member

DHowett commented Jan 27, 2022

@msftbot merge this in 5 minutes

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 27, 2022
@ghost
Copy link

ghost commented Jan 27, 2022

Hello @DHowett!

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 Thu, 27 Jan 2022 23:36:13 GMT, which is in 5 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".

@ghost ghost merged commit f854988 into main Jan 27, 2022
@ghost ghost deleted the dev/migrie/f/elevate-in-sui branch January 27, 2022 23:37
@ghost
Copy link

ghost commented Feb 3, 2022

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

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DHowett DHowett DHowett approved these changes

@miniksa miniksa miniksa approved these changes

@carlos-zamora carlos-zamora Awaiting requested review from carlos-zamora

@leonMSFT leonMSFT Awaiting requested review from leonMSFT

@PankajBhojwani PankajBhojwani Awaiting requested review from PankajBhojwani

@lhecker lhecker Awaiting requested review from lhecker

Assignees

No one assigned

Labels

Area-SettingsUI Anything specific to the SUI AutoMerge Marked for automatic merge by the bot when requirements are met Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zadjii-msft @ianjoneill @DHowett @miniksa