Skip to content

Conversation

@zsprackett
Copy link
Contributor

Implements leaderkey:// URL handlers for:

  • Configuration management (config-reload, config-reveal)
  • Window control (activate, hide, reset)
  • Navigation with execute parameter
  • Settings and about dialogs

  Implements leaderkey:// URL handlers for:
  - Configuration management (config-reload, config-reveal)
  - Window control (activate, hide, reset)
  - Navigation with execute parameter
  - Settings and about dialogs
@mikker
Copy link
Owner

mikker commented Nov 16, 2025

Great additions 👍
I'd like to have simple test coverage for this as well.

@mikker mikker added the enhancement New feature or request label Nov 16, 2025
@zsprackett
Copy link
Contributor Author

Great additions 👍 I'd like to have simple test coverage for this as well.

Added tests.

@mikker
Copy link
Owner

mikker commented Nov 17, 2025

This is not testing functionality, it's duplicating code and then testing the duplicate. If anything, this will just give us a false sense of security.

A better pattern might be to extract the parsing to a simple class that returns and action enum. That's very testable - pass the url, return an enum case - and extracts and encapsulates the logic from the delegate as well. Makes sense?

@zsprackett
Copy link
Contributor Author

what about this version?

@mikker
Copy link
Owner

mikker commented Nov 21, 2025

Yes, much better 👏

@mikker mikker merged commit 22f135b into mikker:main Nov 26, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants