Closed latenitefilms closed 4 years ago
I need to do a formal review of this module, and rethinking the way errors are handled and making it more consistent is one of the things I think I need to standardize...
In general, would you prefer an actual lua error (that you could capture yourself by using pcall
) or returning two arguments -- nil
followed by the text of the error?
I'm also thinking that the shorthand for getting attribute values with the call metamethod (e.g. element("title")
as a shorthand for element:attributeValue("AXTitle")
) should return nil
if the attribute doesn't exist (like the attributeValue
method does) rather than an error like the shorthand currently does.
As to the messaging timeout, there already is a method for element:setTimeout
. What might be worth creating a wrapper for is setting it on the system element (currently you have to do hs._asm.axuielement.systemWideElement():setTimeout(...)
).
I need to do a formal review of this module, and rethinking the way errors are handled and making it more consistent is one of the things I think I need to standardize...
In general, would you prefer an actual lua error (that you could capture yourself by using pcall) or returning two arguments -- nil followed by the text of the error?
I'd be interested to hear @randomeizer's thoughts in regards to error messages.
Generally speaking, I think I prefer return nil
and an error message string, rather than having to use pcall
.
As to the messaging timeout, there already is a method for element:setTimeout. What might be worth creating a wrapper for is setting it on the system element (currently you have to do hs._asm.axuielement.systemWideElement():setTimeout(...)).
Huh - that's very handy to know! I've never noticed that before in the code, thanks for letting me know!
We basically don't use the shorthand for element.title
at the moment because of the error message failure. Instead, we use element:attributeValue("AXTitle")
, so yeah, a solution that doesn't throw an error would probably be more useful. It could return an error as a second return value, if it's important to know. Most of the time, we don't care though.
Probably the same for the performAction(...)
- return value or nil, error
is much easier to deal with than pcall
s.
Ok, all methods (i.e. those defined in the module and the dynamic methods for actions and parameterized attributes) now return nil, errorString
if an actual accessibility error occurs. Note that false
may be a valid response for some of these methods, so you'll need to actually check for nil
specifically (or capture 2 return values and check if the second isn't nil
). One exception is hs.axuielement:getAttributeValue
which will return nil
but without an error string if the error is "Requested value does not exist" (any other error, however, will return nil + the error string)
Note that this does not apply when accessing attributes as key'd elements to the userdata (e.g. obj.AXTitle
), setting them as such (e.g. obj.AXTitle = "boo"
), or when accessing AXChildren
as an array (e.g. #obj
or obj[1]
) because these aren't methods (and the metamethods allowing them only allow for 1 (or 0 in the case of __newindex) return values).
I'm closing this, but reopen if you think I've missed something or you feel it's still an open issue.
Just to clarify, is it now getAttributeValue(...)
, or still attributeValue(...)
?
Otherwise, looks fine. I would be ok with the "Requested value does not exist" being returned as the second value, to be honest, unless it makes life more difficult for direct attribute access via __index
.
Sorry, you're correct, it's attributeValue
without the get
.
And sorry if the info about the error message was unclear... For all methods, if an accessibility error occurs, you will receive nil and an error string.
The macOS accessibility api defines a number of error codes, including kAXErrorNoValue
. For most of the macOS accessibility functions, returning this particular error message would indicate a problem of some kind; however for AXUIElementCopyAttributeValue
it is listed specifically as an expected return value when the attribute has no assigned value... in Lua, that's considered nil
, so for hs.axuielement:attributeValue
we specifically treat that specific error as a "valid" response and only return one value -- nil
. Any other accessibility error, however, will return 2 -- nil, errorString
.
So probably the easiest way to test for an error is to check the second return value specifically:
value, err = obj:attributeValue("AXAttribute") -- or obj:performAction("AXPress"), etc.
if err then
-- we got an error string, so check it out
else
-- value actually contains a valid value of some sort, where nil indicates just that -- no value assigned
end
Accessing attributes through the short cuts (e.g. obj.AXAttribute
and obj.AXAttribute = someValue
) don't return an error message because the can't... even if I write the metamethods to return extra values, Lua swallows them because it would inconsistent with accessing a key's value or setting a key to a new value. However, since the action and parameterized attribute helper actually returns a function, we can return as many values as we like when they are invoked.
See the top section of https://github.com/asmagill/hs._asm.axuielement/blob/master/Reference_Core.md for more details on the shortcut methods.
Yeah, no worries, I figured that was the case. I was specifically referring to this comment:
I think you could just return the error in all cases. No need to make it a special case, since if I don't care about the error message, I can just ignore it. The main concern was if an error
was being thrown from attributeValue(...)
@asmagill - It would be good if
axuielement:performAction()
actually returned an error message type if it fails, so that if it fails due to a timeout (for example), we can make the code try again.It might also be worth exposing the
AXUIElementSetMessagingTimeout
on a per-element basis?