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

Address issues raised by recent reviews #89

Merged
merged 23 commits into from Mar 28, 2023
Merged

Conversation

aphillips
Copy link
Contributor

@aphillips aphillips commented Jan 27, 2023

editor's copy locations include the fragment reference and are linked to @aphillips's repo version, since that displays more nicely than the diff/preview)

Addresses #88
Add guidance for generating bidirectional labels/descriptions
Caused by w3c/i18n-activity#1638
editor's copy location #strcat

Addresses #83
Add guidance on Unicode code point order
editor's copy location #char_sort

Addresses #69
Add information about constituent parts of BCP47 (specifically 4647)
editor's copy location #lang_bcp_not_rfc
editor's copy location #lang_matching_bcp

Addresses #14
Add guidance on using the ILSTR instead of making your own list of subtags
editor's copy location #use_lstr


Preview | Diff

@aphillips aphillips requested review from xfq and r12a January 27, 2023 18:39
@aphillips aphillips self-assigned this Jan 27, 2023
Add guidance on Unicode code point order.
@aphillips aphillips changed the title Add guidance for generating bidirectional labels/descriptions Address issues raised by recent reviews Jan 27, 2023
Add a reference to 4647 when talking about BCP47

Also moved requirements above explanation per our standard (thanks
@r12a)
Mention of using the language subtag registry instead of making your own
list.

Also some cleanup of the language tagging of the Arabic examples.
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
This resulted in a bit of a rewrite.

In addition, I added links to the "choosing a language tag" article,
Richard's language subtag lookup tool, and the registry itself.
@aphillips aphillips requested a review from r12a January 30, 2023 16:46
A couple of browsers don't seem to display the "fixed" bidi example
correctly, at least when respec runs on it. Switching to markup, which
is effective.

Also rewrite the "notice how" paragraph to be (hopefully) clearer.
- made the example text larger to make the color mixing clearer
- simplified the description of the mixing problem
- Removed the charmod reqs altogether
- Added a "warning" box about find being hard and linking our
  string-search doc
@xfq
Copy link
Member

xfq commented Feb 7, 2023

I noticed that Netlify wasn't working. It seems like someone revoked access to all W3C repos (not just the i18n ones). I'll try to add it back.

- Removed extreneous link to specdev?
- Described matching in terms of ranges rather than subtags (4647 is not
  about matching subtags)
- Cleaned up description of 5656, including mentioning the LSTR
Made the example clear that the code units are in the UTF-16 character
encoding.
- Added a requirement not to concat strings!
- Added a "seealso" suggested by @r12a
- Added explanation of the Note example
- Minor wording changes in the colorful example.
@netlify
Copy link

netlify bot commented Feb 9, 2023

Deploy Preview for bp-i18n-specdev ready!

Name Link
🔨 Latest commit 6ce0aea
🔍 Latest deploy log https://app.netlify.com/sites/bp-i18n-specdev/deploys/64135070ca95a60008fc72d0
😎 Deploy Preview https://deploy-preview-89--bp-i18n-specdev.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

The example note should be preceeded by the explanation.
- add exampleChar style (borrowed from our other specs)
- renamed bidi_gen to strcat
- clarified further the concat section
- added links to uts10 and icu user guide for sorting
- moved langtag matching to its own section and fixed label
local.css Outdated Show resolved Hide resolved
@aphillips aphillips requested a review from xfq February 28, 2023 15:31
@aphillips
Copy link
Contributor Author

@r12a Could you look at this for merging-readiness? I'd like to include these changes when pushing to TR...

@aphillips aphillips merged commit de5bee9 into w3c:gh-pages Mar 28, 2023
5 checks passed

<p class="reviewComments"><a href="https://github.com/w3c/i18n-activity/labels/t%3Astrcat" target="_blank">See related review comments.</a></p>

<div class="req" id="strcat_anti_pattern">
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think people will find this general advice about composite messages, that has nothing to do with text direction.

I think it is best to move the section "Composite Strings" under 'Text-processing" and then add a 'see also' to the text direction section 3.6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pending telecon discuss

</div>

<div class="req" id="bidi_gen_advice">
<p class="advisement">Specifications SHOULD provide guidance to implementers on avoiding problems with text directionality for any field(s) that the specification requires an implementation to create or generate for display to users.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather a complicated and long sentence.

How about:
When a specification requires an implementation to create or generate text which will be displayed to users, the specification should provide implementers with guidance on
how to avoid potential problems related to text direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<p class="advisement">Specifications SHOULD provide guidance to implementers on avoiding problems with text directionality for any field(s) that the specification requires an implementation to create or generate for display to users.</p>
</div>

<p>Specifications for APIs, protocols, or document formats sometimes require an implementation to create or provide a field containing a display name or description. When such a string is assembled from separate parts, it can result in problems with presentation or understanding due to the way that the <cite>Unicode Bidirectional Algorithm</cite> [[UAX9]] processes the assembled string. In such cases, the specification should guide implementers about how to create values that will display properly.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Several things are unclear to me here, and rather than provide text suggestions that may be barking up the wrong tree, we should perhaps discuss during our telecon. Questions include:

  • do you mean bidrectional text or text that generally runs in a particular direction (eg. RTL)?
  • i suspect, but i'm not sure, that we are talking solely about strings that contain bidirectional text within them, or about correctly displaying any string that is inserted into a target location by the consumer?
  • if the latter, we're focusing on what producers need to do, we should mention dirname or using other metadata

Basically, i think we need a little more clarity about the role of producers vs consumers, and in-string direction vs how a string will be inserted into a target location. It seems a little vague at the moment. The section title, Concatenation of Strings, doesn't make it sound like we're concerned with string internal direction, but rather strings being inserted into the target location. The example shows one scenario that fits the section title, but the preceding text needs to be clearer before we get to the example, and needs to clarify whether we are also talking about string-internals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added to the agenda for 2023-03-30

</ul>
</aside>


<p>Applications often need to organize sets of information or content. Frequently this involves sorting the content so that users can find what they are looking for. Many data types, such as numbers or dates, are easily sorted by comparing the values. When it comes to textual information, however, the nature of character encodings brings some additional complexity.</p>
<p>Applications often need to organize sets of information or content. Frequently this involves sorting the content. Many data types, such as numbers or dates, are easily sorted by comparing the values. When it comes to textual information, however, the nature of character encodings and user expectations regarding "alphabetical" order brings some additional complexity.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Numbers and dates are only easy to sort if you compare internal representations of the data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done with:

Suggested change
<p>Applications often need to organize sets of information or content. Frequently this involves sorting the content. Many data types, such as numbers or dates, are easily sorted by comparing the values. When it comes to textual information, however, the nature of character encodings and user expectations regarding "alphabetical" order brings some additional complexity.</p>
Many non-textual data types, such as numbers or dates, can be easily sorted using the internal data representation.

@@ -2828,6 +2948,12 @@ <h3>Specifying sort and search functionality</h3>
<p><a href="https://www.w3.org/TR/charmod/#sec-CollationUnits">Units of collation, C008</a>, in <cite>Character Model for the World Wide Web: Fundamentals</cite></p>
</details>
</div>
-->

<aside class="warning" id="find-is-hard" title="String search is hard">
Copy link
Contributor

Choose a reason for hiding this comment

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

As a warning about string searching this seems a little oddly placed here. I think that similar difficulties apply to matching strings for sorting, but i think it would be useful to couch this warning in those terms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section is (and has historically been) called "specifying sort and search functionality". All of the prose in the section is about sorting. This is the only mention of "search"--I added it so that we wouldn't have a gap. I think maybe a subsection div above it would help. I also retitled as "under construction". See the new PR

@@ -431,3 +431,9 @@ table.whitespace caption {
background-color: #04AA6D;
color: white;
}

td.exampleChar {
font-size: 36pt;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a rem or, if necessary, an em unit rather than pt? If you really want to fix the size, perhaps use px rather than pt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Made it 300%

@r12a r12a mentioned this pull request Mar 29, 2023
aphillips added a commit to aphillips/bp-i18n-specdev that referenced this pull request Mar 29, 2023
- Fix style to relative size
- Reword guidance to implementers
- Reword about sorting numbers and dates to say "internal
  representation"
- Added a subsection for "searching" and put up the "under construction"
  banner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants