Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Encoding/Decoding AppNotificationBuilder arguments #2805
Encoding/Decoding AppNotificationBuilder arguments #2805
Changes from 9 commits
d200a18
8954795
20997db
af02980
0a2f8e5
244713d
5d1c7e0
2a3c9be
739fb7c
4e68b51
c50d93f
b4fae36
20a6047
c61056c
be909e5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use tokenizers here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can only be one '=' per token. Find is simpler/better here since we aren't expecting more than 1 delimiter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is it expected to return empty string as the value for a pair? Can we get away with not putting in anything here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Result is a pair of <hstring, hstring. As fas I know there is anything else we can put in this to indicate no value. insert() insists on an hstring won't accept a nullptr.
And since this is part of the IDL, we can really change it to an optional<>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you can use optional in midl 3, but I think it's fine to have an empty "" value. Developers doing AddArgument(L"Key", L""), is valid. The toolkit allows this behavior as well. In the future, we can add AddArgument(key) for a more explicit "I want an empty value", but even then we would store it as { L"key", L"" }.