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

Add macos interaction.pressKeys #37

Merged
merged 12 commits into from
Feb 21, 2024
Merged

Add macos interaction.pressKeys #37

merged 12 commits into from
Feb 21, 2024

Conversation

mzgoddard
Copy link
Contributor

  • Add loadOsModule to load interaction and session modules depending on process.platform
  • Define modules and command types in lib/modules/types.js
  • Move existing interaction and session modules into lib/modules/win32
  • Add lib/modules/macos
  • Add lib/helpers/macos with types and functions to parse key combination into an applescript and execute it with osascript

@mzgoddard mzgoddard requested a review from jugglinmike January 31, 2024 01:44
@mzgoddard mzgoddard force-pushed the macos-key-code-commands branch from c27b6a2 to 070d931 Compare February 12, 2024 17:45
Copy link
Contributor

@jugglinmike jugglinmike left a comment

Choose a reason for hiding this comment

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

Years ago, in the context of the "harness" project, we agreed that unverifiable structured comments degrade the quality of a codebase. Seeing the same patterns being introduced here in this project, my preference is to extend the automated validation process, but I'd settle for documentation that describes how contributors can perform the validation manually. Barring either of those solutions, we should remove the comments.

lib/helpers/load-os-module.js Show resolved Hide resolved
lib/helpers/macos/applescript.js Outdated Show resolved Hide resolved
lib/helpers/macos/applescript.js Outdated Show resolved Hide resolved
lib/helpers/macos/applescript.js Outdated Show resolved Hide resolved
test/helpers/macos/applescript.js Outdated Show resolved Hide resolved
test/helpers/macos/applescript.js Outdated Show resolved Hide resolved
@mzgoddard mzgoddard force-pushed the macos-key-code-commands branch from 37760de to a18dc51 Compare February 15, 2024 17:57
@mzgoddard
Copy link
Contributor Author

@jugglinmike, I made these changes:

  • Rebased onto main and resolved conflict in package.json
  • Added await to retryOnTimeout test
  • Added retryOnTimeout test that forwards non timeout error
  • Turned testEach into suites with individual test('name', () => { per testEach member in test/helpers/macos/applescript.js
  • Throw in retryOnTimeout if it retries too many times
  • Replace Applescript.Script.script with .source
  • Separated renderScript template string into subfunctions and variables
  • Fixed @type uses to get Typescript to type check

Copy link
Contributor

@jugglinmike jugglinmike left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, @mzgoddard; I got sidetracked by a test failure which is also occurring on the main branch. Now that I know this patch is unrelated, we can merge it and take on that problem separately.

(By the way, the code style also doesn't validate on main--we really need to get CI set up!)

@jugglinmike jugglinmike merged commit 5aac12e into main Feb 21, 2024
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