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

[khmer_ldml] new Khmer LDML keyboard #3273

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Nnyny
Copy link
Contributor

@Nnyny Nnyny commented Jan 9, 2025

No description provided.

@Nnyny Nnyny marked this pull request as draft January 9, 2025 09:00
@keyman-server
Copy link
Collaborator

Thank you for your pull request. You'll see a "build failed" message until the Keyman team has reviewed the pull request and manually initiated the build process.

Every change committed to this branch will become part of this pull request. When you have finished submitting files and are ready for the Keyman team to review this pull request, please post a "Ready for review" comment.

PLEASE NOTE: team capacity for review is limited in December 2024 and will resume in January 2025. We appreciate your patience.

@Nnyny Nnyny changed the title [khmer_test] new Khmer LDML keyboard [khmer_ldml] new Khmer LDML keyboard Jan 27, 2025
@Nnyny Nnyny marked this pull request as ready for review January 27, 2025 06:58
srl295
srl295 previously approved these changes Feb 4, 2025
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

  • LDML looks good
  • passes the previous regression tests

LGTM, great work.

@LornaSIL
Copy link
Contributor

LornaSIL commented Feb 4, 2025 via email

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

This is looking good! I have a few further change requests -- all minor. The name khmer_ldml is not ideal from an end user perspective because it's talking about the technology rather than the feature of the keyboard. Perhaps it should be called "Khmer Smart" or something that emphasizes the automatic fixups it does?

experimental/k/khmer_ldml/khmer_ldml.user Outdated Show resolved Hide resolved
experimental/k/khmer_ldml/source/khmer_ldml.kps Outdated Show resolved Hide resolved
experimental/k/khmer_ldml/LICENSE.md Outdated Show resolved Hide resolved
experimental/k/khmer_ldml/source/khmer_ldml.kps Outdated Show resolved Hide resolved
experimental/k/khmer_ldml/HISTORY.md Outdated Show resolved Hide resolved
experimental/k/khmer_ldml/source/readme.htm Outdated Show resolved Hide resolved

<h1>Keyboard Layout</h1>

<!-- Insert Keyboard Layout Images or HTML here -->
Copy link
Member

Choose a reason for hiding this comment

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

This is missing user documentation; I know it's in experimental so it'll be okay, but it won't be able to progress to a release keyboard until that is resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I use the one from Khmer Angkor keyboard?

Copy link
Member

Choose a reason for hiding this comment

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

yes that'll be fine

experimental/k/khmer_ldml/source/khmer_ldml.kps Outdated Show resolved Hide resolved
@Nnyny
Copy link
Contributor Author

Nnyny commented Feb 5, 2025

@mcdurdin, so are we changing the name to "Khmer Smart" or is this need more consideration?

@mcdurdin
Copy link
Member

mcdurdin commented Feb 5, 2025

need more consideration?

We should discuss with @MakaraSok for sure!

@mcdurdin
Copy link
Member

mcdurdin commented Feb 5, 2025

e.g. khmer_angkor_thmey

@mcdurdin
Copy link
Member

mcdurdin commented Feb 5, 2025

khmer_thmey_thmey

@Nnyny
Copy link
Contributor Author

Nnyny commented Feb 5, 2025

User documentation will be updated later in another PR. Please review the changes.

@DavidLRowe
Copy link
Contributor

For the "experimental" section, we don't require user documentation.

@DavidLRowe
Copy link
Contributor

I'm not qualified to comment on the actual function of the Khmer keyboard, but everything else looks good, except that the "Welcome file:" field ("Details" tab for "Packaging") should point to the "welcome.htm" file. [This field should have been supplied by the New Project function. See keyman issue #13135.]

I could try making that change, but since you will likely change the keyboard project name, I'll let you make that change to the packaging file.

Congratulations!

@Nnyny
Copy link
Contributor Author

Nnyny commented Feb 7, 2025

Thank you! I'll look into that missing file.

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.

7 participants