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

Update app.exe matching to work with both public and beta Talon #1385

Merged
merged 10 commits into from
Feb 19, 2024

Conversation

nriley
Copy link
Collaborator

@nriley nriley commented Feb 10, 2024

  • Update mixed-case app.exe: matchers to work with public and beta Talon.
  • Update app switcher to work with public and beta Talon.

To fix talonvoice/talon#355, beta Talon uses
lowercase for app.exe. Any app.exe matchers that use capital letters
won't work on beta Talon, so temporarily change them to case-insensitive
regular expression matchers until the next public release of Talon
matches the beta's behavior.
To fix talonvoice/talon#355, beta Talon uses
lowercase for app.exe. As the app name overrides CSV allows you to
override by app.exe on Windows, any overrides (including some defaults
from community) will won't work on beta Talon, so lowercase everything.

Also updates "running list" to be a more readable and implements the
"todo" so overrides actually override spoken forms for an application.
@nriley
Copy link
Collaborator Author

nriley commented Feb 10, 2024

I've not got Windows at home so this specific set of changes aren't well tested, but wanted to get it out there since these changes will affect Talon beta users. Will test more on Monday if nobody else does over the weekend!

nriley and others added 4 commits February 17, 2024 11:49
Don't clear app name overrides when other files in its directory change.

Also update file name matching for Talon returning Windows file paths as lowercase.
@nriley
Copy link
Collaborator Author

nriley commented Feb 17, 2024

Been using this all week and made a couple more changes as well as fixing a bug I was surprised was there so long (the override list was getting cleared entirely!) Also since I had been using "running list" more I made some changes to allow you to exclude applications entirely from it to make the list more manageable (you can see my list here: https://github.com/nriley/talon_community/blob/nriley/core/app_switcher/app_name_overrides.windows.csv)

@nriley nriley marked this pull request as ready for review February 17, 2024 16:53
@knausj85
Copy link
Member

Should we update the copy context commands to do this regex magic?

@nriley
Copy link
Collaborator Author

nriley commented Feb 17, 2024

Should we update the copy context commands to do this regex magic?

Good call, yes! Will do (update: done, see below).

To fix talonvoice/talon#355, beta Talon uses
lowercase for app.exe. Any app.exe matchers that use capital letters
will fail on beta Talon, so temporarily generate case-insensitive
regular expression matchers.

Also use more concise method to extract the last component of a path.
@AndreasArvidsson
Copy link
Collaborator

Definitely. We should also update create app context:

return f"app.exe: {active_app.exe.split(os.path.sep)[-1]}"

We should probably even unify the implementation of these two

To fix talonvoice/talon#355, beta Talon uses
lowercase for app.exe. Any app.exe matchers that use capital letters
will fail on beta Talon, so temporarily generate case-insensitive
regular expression matchers.

Also use more concise methods to extract the last component of a path
and strip trailing ".exe".
@nriley
Copy link
Collaborator Author

nriley commented Feb 18, 2024

Definitely. We should also update create app context:

return f"app.exe: {active_app.exe.split(os.path.sep)[-1]}"

Done.

We should probably even unify the implementation of these two

Agreed, although would prefer to do another time. They generate Windows app matches differently — create_app_context just matches app.exe and talon copy context also matches app.name.

Copy link
Member

@knausj85 knausj85 left a comment

Choose a reason for hiding this comment

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

This look solid to me with one minor comment

@knausj85 knausj85 merged commit 520b106 into main Feb 19, 2024
2 checks passed
@knausj85 knausj85 deleted the lowercase-app-exe branch February 19, 2024 01:04
Mark-Phillipson pushed a commit to Mark-Phillipson/community that referenced this pull request Mar 29, 2024
…nhub#1385)

- Update mixed-case app.exe: matchers to work with public and beta
Talon.
- Update app switcher to work with public and beta Talon.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
sisi-sh added a commit to sisi-sh/sisi_talon that referenced this pull request Apr 1, 2024
* 'main' of github.com:talonhub/community: (73 commits)
  add lowercased firefox-esr to firefox.py (talonhub#1410)
  configurable number prefix without mouse grid (talonhub#1342)
  Update app.exe matching to work with both public and beta Talon (talonhub#1385)
  Add comment line to javascript (talonhub#1387)
  Add detail to documentation (talonhub#1380)
  Switch to black pre-commit mirror (talonhub#1378)
  Update app_name_overrides.windows.csv to use Windows Terminal instead of iTerm2 (talonhub#1379)
  [pre-commit.ci] pre-commit autoupdate (talonhub#1376)
  coalesce some list updates (talonhub#1348)
  Remove unused setting from c code language (talonhub#1368)
  mouse: adding synonyms for starting left drag. (talonhub#1356)
  Adds commands to focus on the last application (talonhub#1336)
  Stop using “brackets” to mean “braces” (talonhub#1344)
  Update settings descriptions (talonhub#1371)
  Revert changes to `user.mouse_enable_pop_click` (talonhub#1370)
  Fix typo in `mouse.py` variable name (talonhub#1372)
  Convert boolean settings to boolean (talonhub#1360)
  Add ruby op-or-equals command (talonhub#1347)
  added app name Mate-terminal (fork of Gnome-terminal) (talonhub#1367)
  added app name Caja (fork of Nautilus) (talonhub#1366)
  ...
emragins pushed a commit to emragins/talon-community that referenced this pull request May 28, 2024
…nhub#1385)

- Update mixed-case app.exe: matchers to work with public and beta
Talon.
- Update app switcher to work with public and beta Talon.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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

4 participants