Skip to content

CLDR-17829 move PathDescriptions.txt to PathDescriptions.md #4633

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

Merged
merged 13 commits into from
Apr 24, 2025

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Apr 23, 2025

  • add lychee build task to link check PathDescriptions.md
  • added PathDescriptionParser and unit tests
  • fixes many descriptions, link targets, and placeholders

CLDR-17829

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

- add lychee build task to link check.
- added PathDescriptionParser and unit tests
- fix unneeded HTML escaping
- add cldrDom.setDocTargets() so the server doesn't need to munge content
- update test
@srl295 srl295 marked this pull request as ready for review April 23, 2025 20:40
@srl295
Copy link
Member Author

srl295 commented Apr 23, 2025

Note: hard build failure if PathDescriptions.md contains an invalid link: (note cldr.unicode.xrg)

image

- fix site reference
- remove old todos
description = MISSING_DESCRIPTION;
} else if ("SKIP".equals(description)) {
Copy link
Member

Choose a reason for hiding this comment

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

If it starts with SKIP, but has other characters, it is an error; so best to raise an exception.

Copy link
Member Author

Choose a reason for hiding this comment

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

the description here is more rich, it's for you to name (or in the future group) these entries.

as below, where First Day of the Week is a comment.

### First Day of the Week

- `^//ldml/localeDisplayNames/types/type\[@key="fw"]\[@type="([^"]*)"]`

The name of “first day of the week is {1}”. For more information, please see [Key Names].

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand ... that snippet does not have SKIP in it

Copy link
Member

@macchiati macchiati left a comment

Choose a reason for hiding this comment

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

Mostly minor except for the "#h."

…/util/data/PathDescriptions.md

Co-authored-by: Mark Davis <mark@unicode.org>
@srl295
Copy link
Member Author

srl295 commented Apr 23, 2025

@macchiati some of the other targets don't seem to exist.

@srl295
Copy link
Member Author

srl295 commented Apr 23, 2025

"Cyclic Name Sets" isn't an anchor (defect in date-time-names.md )

@srl295
Copy link
Member Author

srl295 commented Apr 23, 2025

I thought this task was to move the format over, for future fixup. I will revisit this tomorow

@srl295
Copy link
Member Author

srl295 commented Apr 23, 2025

I'll add a test against the relevant sites page… Probably for a future PR

@macchiati
Copy link
Member

I thought this task was to move the format over, for future fixup. I will revisit this tomorow

I just thought that as long as you were getting the anchor text, you could fix the links as well. It can be a separate PR though.

@srl295
Copy link
Member Author

srl295 commented Apr 24, 2025

I thought this task was to move the format over, for future fixup. I will revisit this tomorow

I just thought that as long as you were getting the anchor text, you could fix the links as well. It can be a separate PR though.

No, i had just title cased the variable names.

@srl295 srl295 requested a review from macchiati April 24, 2025 15:40
@srl295
Copy link
Member Author

srl295 commented Apr 24, 2025

All #h. fragments removed and corrected as best I can.

KEY_NAMES / [Key Names] has been broken for a couple of years according to git. A bunch of the types go to the same section on sensitive territory names.

    public static final String KEY_NAMES =
            "https://cldr.unicode.org/translation/displaynames/countryregion-territory-names#h.x27jspwj91af";

So probably all of these need review.

macchiati
macchiati previously approved these changes Apr 24, 2025
- copy changes from unicode-org#4630 into PathDescriptions.md
- fix broken placeholders
- correct CSS issue
@macchiati
Copy link
Member

I was thinking that for this file and similar ones, it would be useful to have a tool that generated a sample of each message, that would be used to build a web page; we could then review to make sure that all the parameters were right, etc.

Not part of this ticket — I'll file a different one for it.

@macchiati
Copy link
Member

https://unicode-org.atlassian.net/browse/CLDR-18555

macchiati
macchiati previously approved these changes Apr 24, 2025
srl295 added 2 commits April 24, 2025 14:06
- add test that no placeholders leak through
- fix additional paths
- test was failing because URLs are now in the references instead of inline
@srl295
Copy link
Member Author

srl295 commented Apr 24, 2025

sorry, i was running the wrong test locally.

This also now fixes a number of cases where the placeholders {} were wrong and were being passed through to users. and adds a test so it doesn't happen again.

@srl295
Copy link
Member Author

srl295 commented Apr 24, 2025

I was thinking that for this file and similar ones, it would be useful to have a tool that generated a sample of each message, that would be used to build a web page; we could then review to make sure that all the parameters were right, etc.

Not part of this ticket — I'll file a different one for it.

i recommend deduping cases where the message is the same for multiple xpaths.

@srl295 srl295 requested a review from AEApple April 24, 2025 20:15
@macchiati
Copy link
Member

I was thinking that for this file and similar ones, it would be useful to have a tool that generated a sample of each message, that would be used to build a web page; we could then review to make sure that all the parameters were right, etc.
Not part of this ticket — I'll file a different one for it.

i recommend deduping cases where the message is the same for multiple xpaths.

That's what I meant by one message per line, eg per regex path in the <key, values> pair.

@srl295 srl295 merged commit 22a1909 into unicode-org:main Apr 24, 2025
14 checks passed
@srl295 srl295 deleted the cldr-17829/pathheader-markdown branch April 24, 2025 20:41
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.

2 participants