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

feat: Add tool to generate gesture table #2594

Closed
wants to merge 2 commits into from

Conversation

darcywong00
Copy link
Contributor

@darcywong00 darcywong00 commented Mar 7, 2024

Addresses #2213

This adds a gestureChart tool to document gestures in a touch layout as a Markdown table. Future TODO is incorporating the Markdown table into the welcome.htm / .php help docs. If we sync the .md file along with the .php help, the site could render it as
image

As of 17.0, gestures can be:

  • longpress
  • flicks
  • multitap

Usage

node gestureChart.js -k <keyboard>
    -k, --keyboard <full path to keyboard directory>

Sample outputs:
Current sil_ipa gestures (on master)
sil_ipa-tablet (current).md

sil_ipa incorporating gestures from #2521
sil_ipa-tablet.md

rawang from #2554
rawang-tablet.md

text = id.substring(2);
} else {
text = `[${id}]`;
console.warn(`id ${id} doesn't start with U_`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking up text from a [T_XXXX] key in the .js keyboard file is an extra complexity

Copy link
Member

Choose a reason for hiding this comment

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

It's also not needed -- we don't support inferring text for T_ keys in the touch layout 😁 -- so this is the correct approach I think

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 a useful tool. However, we don't want to start putting tooling into the keyboards repository, because this is really a Developer tool.

So, we need to move the tool to keymanapp/keyman, and ideally we will integrate this into kmc as a kmc module. Then we could merge it in 18.0.

If that sounds like too much work for now, then feel free to put it into its own repo -- just we shouldn't keep it in keyboards repo.

One other challenge is that while this tool generates Markdown, we don't currently support Markdown in keyboard documentation welcome.htm, which is where this content belongs (not just on help.keyman.com).

The whole Markdown documentation proposal can be followed in keymanapp/keyman#9477

Comment on lines +180 to +182
if (key.text.startsWith('|')) {
// Handle vertical line character
text += key.text.replaceAll('|', '&#124;');
Copy link
Member

Choose a reason for hiding this comment

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

This will not work with x| or things like that. Nor |*. We need to do replacements on the whole string. There's actually a escapeMarkdownChar function in Developer which should do what is needed:

https://github.com/keymanapp/keyman/blob/5062bbad984760a40e5ee3d012b65d54d805db1d/developer/src/common/web/utils/src/markdown.ts#L2-L21

text = id.substring(2);
} else {
text = `[${id}]`;
console.warn(`id ${id} doesn't start with U_`);
Copy link
Member

Choose a reason for hiding this comment

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

It's also not needed -- we don't support inferring text for T_ keys in the touch layout 😁 -- so this is the correct approach I think

@mcdurdin
Copy link
Member

mcdurdin commented Mar 7, 2024

One other small note, if we do move to keyman repo, then TouchLayoutFileReader should be used to read the file, because that does validation and normalization of content

@darcywong00
Copy link
Contributor Author

This is a useful tool. However, we don't want to start putting tooling into the keyboards repository, because this is really a Developer tool.

So, we need to move the tool to keymanapp/keyman, and ideally we will integrate this into kmc as a kmc module. Then we could merge it in 18.0.

If that sounds like too much work for now, then feel free to put it into its own repo -- just we shouldn't keep it in keyboards repo.

Understood. For now, I'll move this to its own repo (so I can continue to use on an individual keyboard basis).
I'll then incorporate all these review comments as keymanapp/keyman#10965

@darcywong00
Copy link
Contributor Author

This is moved to a separate repo (keymanapp/gestureChart) and comments to be resolved in keymanapp/keyman#10965

@darcywong00 darcywong00 closed this Mar 8, 2024
@mcdurdin
Copy link
Member

mcdurdin commented Mar 8, 2024

Sounds great! I like this tool, and it's going to be helpful in our updated documentation approach in the future too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants