Skip to content
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

Include input & option descriptions as data in registry #533

Merged
merged 5 commits into from
Dec 11, 2023

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Nov 27, 2023

One rather important consumer of the registry is syntax highlighting. For that use, we should include the descriptions of the inputs and options in the actual registry data, rather than just in the comments. And the element bodies are a rather excellent place for that.

@eemeli eemeli added the registry Issue pertains to the function registry label Nov 27, 2023
@eemeli eemeli requested review from aphillips and stasm November 27, 2023 14:21
@eemeli eemeli force-pushed the option-descriptions branch from a266e97 to b9cbbc6 Compare November 27, 2023 16:06
Copy link
Member

@aphillips aphillips left a comment

Choose a reason for hiding this comment

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

I like the general direction. Some comments (mainly editorial).

<option name="timeStyle" values="full long medium short">
The predefined time formatting style to use.
</option>
<option name="calendar" values="buddhist chinese coptic dangi ethioaa ethiopic gregory hebrew indian islamic islamic-umalqura islamic-tbla islamic-civil islamic-rgsa iso8601 japanese persian roc">
Copy link
Member

Choose a reason for hiding this comment

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

Which source did you use for these? We should have a pointer as this list is not likely to be stabilized.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only moved the description into the body here; the values were added by @mihnita in #420; I presume that their source is the ECMA-402 spec, which may itself refer to another source.

Copy link
Member

Choose a reason for hiding this comment

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

I know. But once they move from comments to data, we should spend some extra effort on it 😉

<option name="calendar" values="buddhist chinese coptic dangi ethioaa ethiopic gregory hebrew indian islamic islamic-umalqura islamic-tbla islamic-civil islamic-rgsa iso8601 japanese persian roc">
Calendar to use.
</option>
<option name="numberingSystem" values="arab arabext bali beng deva fullwide gujr guru hanidec khmr knda laoo latn limb mlym mong mymr orya tamldec telu thai tibt">
Copy link
Member

Choose a reason for hiding this comment

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

This list is incomplete? Can we point to CLDR data here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Again, I didn't change the values, only moved the description from the comment to the the contents.

Copy link
Member

Choose a reason for hiding this comment

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

ibid

spec/registry.xml Outdated Show resolved Hide resolved
Comment on lines +2 to +4
<!ATTLIST registry
xml:lang NMTOKEN #IMPLIED
>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added xml:lang as an optional attribute on the top-level <registry>, as well as directly on <input> and <option>, to indicate the language of the included descriptions.

Should this be required, though? Or are there use cases where a <registry> could be e.g. embedded in another XML document, where the locale is defined somewhere further up the tree?

Copy link
Member

Choose a reason for hiding this comment

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

We should do this differently. Literally in the next tab of the browser I have a PR open on string-meta with best practices. See also this article.

I think we'd be best off here by putting the natural language string descriptions into sub-elements description which are allowed to vary by xml:lang. Setting xml:lang="en" at the registry element level would allow us to have <description> without xml:lang attributes throughout.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Many <desciption> now.

@eemeli eemeli requested a review from aphillips December 8, 2023 06:48
Copy link
Member

@aphillips aphillips left a comment

Choose a reason for hiding this comment

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

sheep it.

🐑

@aphillips aphillips merged commit 866a71a into unicode-org:main Dec 11, 2023
1 check passed
@eemeli eemeli deleted the option-descriptions branch December 11, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
registry Issue pertains to the function registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants