-
Notifications
You must be signed in to change notification settings - Fork 79
Description
Background / Problem
In #29, we added generated sample options to the sample system. This included a multiple choice option, where due to the limitations of attributes, the syntax for a single multi-choice option looks something like:
[ToolkitSampleMultiChoiceOption("TextForeground", label: "Teal", value: "#0ddc8c", title: "Text foreground")]
[ToolkitSampleMultiChoiceOption("TextForeground", label: "Sand", value: "#e7a676")]
[ToolkitSampleMultiChoiceOption("TextForeground", label: "Dull green", value: "#5d7577")]There's a lot of room for improvement here. @michael-hawker and I had a brief discussion in the UWPCommunity Discord (starting here, ending here) on ways that we can improve on it.
Solution
Since attributes lack the ability to hold any data structure that resembles a key-value pair, and since Source generators allow us to do custom validation of data given to attributes, we eventually landed on the following syntax:
[ToolkitSampleMultiChoiceOption(propname: "TextForeground", title: "Text foreground", "Teal : #0ddc8c", "Sand : #e7a676", "Dull green : #5d7577")]
// With different formatting.
[ToolkitSampleMultiChoiceOption(propname: "TextForeground", title: "Text foreground",
"Teal : #0ddc8c",
"Sand : #e7a676",
"Dull green : #5d7577")]
// With different formatting.
[ToolkitSampleMultiChoiceOption("TextForeground", title: "Text foreground",
choices: new string[] { "Teal : #0ddc8c", "Sand : #e7a676", "Dull green : #5d7577" })]The goal was to replace the function of a ValueTuple without making things overly complicated, while still closely pairing the label to the value (e.g. not using 2 separate string[]).
We landed on using :, a colon surrounded by a space on both sides, as a separator for a key-value pair.
We'll need to
- Refactor the source generators to use this
- Refactor the existing samples to use this
- Gut any custom diagnostic generators and unit tests for the old syntax
- Add new diagnostic generators to ensure each string is properly formatted.
- Trim whitespace from the end of each label, and from the start of each value (allows for custom alignment)
- Add an optional flag to disable whitespace trimming, so values that require leading whitespace can be used if needed. The use cases here are very few at first glance, but not nonexistent. Should be an easy add.