Conversation
ghost
added
Area-Settings
This comment has been minimized.
This comment has been minimized.
carlos-zamora
force-pushed
the
dev/cazamor/set/winrt-app-obj-top
branch
from
7d78e47 to
700290e
Compare
| if (equals(profileGuid, guid)) | ||
| { | ||
| std::iter_swap(_profiles.begin() + pIndex, _profiles.begin() + gIndex); | ||
| auto prof1 = _profiles.GetAt(pIndex); |
There was a problem hiding this comment.
i bet there's a way to use iterators here ;P
There was a problem hiding this comment.
So, the problem is more the +pIndex and +gIndex. When I do this:
std::iter_swap(_profiles.First() + pIndex, _profiles.First() + gIndex);there isn't a + operator found :(
There was a problem hiding this comment.
Ugh, there's a bunch of iterator helpers that are private. Bah.
ghost
added
Needs-Author-Feedback
This comment has been minimized.
This comment has been minimized.
| _profiles.end(), | ||
| [](auto&& profile) { return profile.Hidden(); }), | ||
| _profiles.end()); | ||
| for (uint32_t i{}; i < _profiles.Size();) |
There was a problem hiding this comment.
Oh, please just assign 0 to i. I know the {} will technically default construct it to 0, but that's so much more obtuse than the thing everyone recognizes.
There was a problem hiding this comment.
ha! you know, i thought you were part of our discussions about zero-initializing things with {}, and perhaps with the ayes and not the nays
miniksa
left a comment
miniksa
left a comment
There was a problem hiding this comment.
Looks like it does what it says on the box. I'd prefer to have Dustin also sign, though, as he has a bit more WinRT experience and sees things I don't.
carlos-zamora
force-pushed
the
dev/cazamor/set/winrt-app-obj-top
branch
from
641e1c7 to
1885822
Compare
| { | ||
| auto newSettings = _isUwp ? CascadiaSettings::LoadUniversal() : CascadiaSettings::LoadAll(); | ||
| _settings.copy_from(winrt::get_self<CascadiaSettings>(newSettings)); | ||
| _settings = newSettings; |
There was a problem hiding this comment.
to save eight nanoseconds you could do uh
| _settings = newSettings; | |
| _settings = std::move(newSettings); |
or just _settings = isUwp ? xxx : yyy
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
|
🎉 Handy links: |
ghost
mentioned this pull request
3 participants
CascadiaSettings is now a WinRT object in the TerminalApp project.
References
#7141 - CascadiaSettings is a settings object
#885 - this new settings object will be moved to a new TerminalSettingsModel project
This one looks big, but most of it is really just propagating the
changes to the tests. In fact, you can probably save yourself some time
because the tests were about an hour of Find&Replace.
CascadiaSettings::GetCurrentAppSettings()was only being used inPane.cpp. So I ripped out the 3 lines of code and stuffed them in there.
Follow-up work:
get_selfto be able to getthe warnings out. This will go away in the next PR (wrapping up Replace TerminalSettings runtimeclasses with interfaces only #885)
Validation Steps Performed
Closes #7141