Skip to content

Conversation

@cinnamon-msft
Copy link
Contributor

@cinnamon-msft cinnamon-msft commented Nov 9, 2020

  • Change DesktopWallpaper to desktopWallpaper for backgroundImage
    to match our other settings
  • Add desktopWallpaper to json schema

static constexpr std::string_view BellStyleKey{ "bellStyle" };

static const winrt::hstring DesktopWallpaperEnum{ L"DesktopWallpaper" };
static const winrt::hstring DesktopWallpaperEnum{ L"desktopWallpaper" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGH
@carlos-zamora can you help make this not a statically-constructed hstring

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 9, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 9, 2020
static constexpr std::string_view BellStyleKey{ "bellStyle" };

static const winrt::hstring DesktopWallpaperEnum{ L"DesktopWallpaper" };
static const winrt::hstring DesktopWallpaperEnum{ L"desktopWallpaper" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static const winrt::hstring DesktopWallpaperEnum{ L"desktopWallpaper" };
static const std::string_view DesktopWallpaperEnum{ L"desktopWallpaper" };

I think that's all you might have to do since to_hstring is called on this when it's needed below.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh this will definitely not work lol

why is it even a wide string

why did we put L on it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, remove the L after making it a string_view

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I think I fixed it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I'm a dummy. Make it a std::wstring_view (and add the 'L' back in).

Since we're specifically comparing this to an hstring and hstrings are wide, keep this as a wstring and remove to to_hstring call when it's used.

Mind fixing it? It'll be a quick fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay how about now?

@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Nov 9, 2020
Comment on lines +760 to +764
{
"enum": [
"desktopWallpaper"
]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double checking, you don't need "type": "string" here? It looks like "backgroundImageAlignment" below has it. Either way, thank you!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i sure hope this works lol

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k

@DHowett
Copy link
Member

DHowett commented Nov 9, 2020

@msftbot merge this in 3 minutes

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Nov 9, 2020
@ghost
Copy link

ghost commented Nov 9, 2020

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 Mon, 09 Nov 2020 22:49:55 GMT, which is in 3 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 e9a7b24 into main Nov 9, 2020
@ghost ghost deleted the cinnamon/fix-desktop-wallpaper-casing branch November 9, 2020 22:50
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AutoMerge Marked for automatic merge by the bot when requirements are met

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants