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

adds hs.hotkey.assigned #1444

Merged
merged 3 commits into from
Aug 17, 2017
Merged

Conversation

asmagill
Copy link
Member

sort of addresses #1391

This isn't really a conclusive test (for example, both Cmd-F5 and Ctrl-1 are defined on my system and will return a table with this function; however, if I create a Hammerspoon hotkey for Cmd-F5, it works, while a Hammerspoon hotkey forCtrl-1 fails.)

What's the group consensus? It's useful to see if you're trying to overwrite a system hotkey, but... it isn't a reliable way to tell if you can or not.

Function: hs.hotkey.assigned(mods, key) -> table | false

Examine whether a potential hotkey is in use by the macOS system such as the Screen Capture, Universal Access, and Keyboard Navigation keys.

Parameters:

  • mods - A table or a string containing (as elements, or as substrings with any separator) the keyboard modifiers required,
    which should be zero or more of the following:
    • "cmd", "command" or "⌘"
    • "ctrl", "control" or "⌃"
    • "alt", "option" or "⌥"
    • "shift" or "⇧"
  • key - A string containing the name of a keyboard key (as found in hs.keycodes.map ), or a raw keycode number

Returns:

  • if the hotkey combination is in use by a system function, returns a table containing the following keys:
    • keycode - the numberic keycode for the hotkey
    • mods - a numeric representation of the modifier flags for the htokey
    • enabled - a boolean indicating whether or not the key is currently enabled
  • if the hotkey combination is not in use by the operating system, returns the boolean value false

Notes:

  • this is provided for informational purposes and does not provide a reliable test as to whether or not Hammerspoon can use the combination to create a custom hotkey -- some combinations which return a table can be over-ridden by Hammerspoon while others cannot. At this time there is no known reliable test except to assign the hotkey with hs.hotkey.enable and see if it works.

@codecov
Copy link

codecov bot commented May 30, 2017

Codecov Report

Merging #1444 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1444      +/-   ##
==========================================
- Coverage   23.65%   23.63%   -0.02%     
==========================================
  Files         127      129       +2     
  Lines       23686    23896     +210     
  Branches     2813     2838      +25     
==========================================
+ Hits         5602     5649      +47     
- Misses      17726    17881     +155     
- Partials      358      366       +8

@cmsj
Copy link
Member

cmsj commented Jun 30, 2017

I don't have any objections, but I wonder if this could be implemented as a wrapper around hs.hotkey.new() that tries to bind the hotkey, and removes it if it succeeded?

@asmagill
Copy link
Member Author

asmagill commented Jul 1, 2017

@cmsj, as noted above, just because this function returns a table doesn't reliably determine whether or not Hammerspoon can override it. If this function returns false, then the hotkey is safe to use... if it returns a table, the hotkey may or may not be safe to use.

Since Hamemrspoon can over-ride some of the combinations this returns a table for, do we really want to prevent it? And if we do add the check, how many existing user setups will break because of it?

At the very least, if you do want the check added, there should be someway to tell .new or .bind to ignore the results of the check.

@cmsj
Copy link
Member

cmsj commented Jul 10, 2017

@asmagill I didn't mean that we would bake this check into .new()/.bind(), more that .assigned() (or maybe .isAvailable() ?) would call .new() and see if it works or not, unbinding it if it did work.

@asmagill
Copy link
Member Author

Because of the heavy additions made to hs.hotkey to handle stacking assignments of the same hotkey and adding the inline labels/help, it's not intuitively obvious, but this can already be done with the existing infrastructure with something like (I've not tested this as a function, but the doing this by hand with a combination that I know works and a combination that I know fails works as expected, so I am fairly confident the final version will be pretty similar):

hotkey.isAssignable = function(...)
    local k = hotkey.new(...)
    local prevLevel = hs.luaSkinLog.level
    -- supress luaSkinLog error if binding fails
    hs.luaSkinLog.level = 0
    local status = k._hk:enable()
    if status then k._hk:disable() end
    hs.luaSkinLog.level = prevLevel
    return status and true or false
end

Should this be added instead? Or are both worth having? The earlier version could still be used to determine if the OS is using the hotkey combination -- just because someone can override some of these doesn't necessarily mean that they will want to.

@asmagill
Copy link
Member Author

@cmsj, clarify your position on my previous comment -- this could be added to #1433 fairly easily.

@cmsj
Copy link
Member

cmsj commented Jul 17, 2017

@asmagill yeah that's the kind of thing I meant :)

@asmagill
Copy link
Member Author

Unless you object, I'm going to move the function which checks against system definitions to hs.hotkey.systemAssigned and add the example function above as hs.hotkey.assigned.

Should hs.hotkey.new (which is already a wrapper to hs.hotkey.bind(...):enable()) do something similar and return nil on failure? This would be a change in behavior because currently new returns a hotkey table object, just one that isn't/can't be enabled if binding fails.

@cmsj
Copy link
Member

cmsj commented Jul 17, 2017

@asmagill hmm, that's a tough one. I think returning nil would definitely be more correct, but nobody is going to be checking the return type, so it might make a lot of things explode.

I think it's probably better to move the API towards being useful though, so let's try it and see what happens :D

@asmagill
Copy link
Member Author

hs.hotkey.systemAssigned and hs.hotkey.assignable added

hs.hotkey.new(...):enable() (also known as hs.hotkey.bind(...)) will return nil if activation fails, but still displays the error messages to the console (assignable suppresses them)... this is because the most common use of hs.hotkey.bind is to simply execute it and ignore the return value.

If you really want to suppress the messages no matter what, either check with hs.hotkey.assignable before binding or wrap your hotkey assignments with:

local prevLevel = hs.luaSkinLog.level
hs.luaSkinLog.level = 0

-- do hotkey assignments as usual

hs.luaSkinLog.level = prevLevel

Note that in the future, it is hoped that these will be sent to a hs.hotkey specific log, so suppressing them can be more local than by temporarily suppressing all error messages coming from objc (hence the reassignment to prevLevel after assignments) but #1425 and #1426 aren't going to be completed in time for the next release.

@asmagill asmagill merged commit 15db725 into Hammerspoon:master Aug 17, 2017
@asmagill asmagill deleted the hotkeyAssigned branch September 15, 2017 18:41
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