Fix direct and indirect type handlers for JXA

[n8henrie]

Fixes https://github.com/quicksilver/Quicksilver/issues/2604

Would love any feedback from @pjrobertson or @skurfer (on anything but variable names, but I guess that's fine too 😆).

[pjrobertson]

Other than a few minor styling suggestions, I don't see any glaring problems with this. Again, the hard-coding of function names (with half a bracket now () isn't ideal, but if it works, then why not? :D

[n8henrie]

Other than a few minor styling suggestions

Would you mine elaborating? I think I have a rudimentary sense of style and preferences in bash and python, and rust and go do some hand-holding with their linters, but this is totally new territory and feels very foreign. For example I was hoping to find a more satisfying way to concatenate the arrays.

Again, the hard-coding of function names (with half a bracket now () isn't ideal

Agreed! If you come across a better solution let me know. As far as I can tell it is identical to what we've been doing with AppleScript, just that AppleScript obfuscates the function names (which is why it also works if you just add the AppleScript handler name in a JavaScript comment, just feels gross). I figure trying to get as specific as a match as possible is the best approach, which is why functions that accept zero arguments include both parentheses, and the rest accept up to the opening parenthesis.

[pjrobertson]

Would you mine elaborating?

Sorry, I wrote them directly on the code again, but I think last time you had trouble seeing them?

[n8henrie]

Sorry, I wrote them directly on the code again, but I think last time you had trouble seeing them?

Yes, they're still not showing up for me. Did you hit the Submit Review button?

[n8henrie]

@pjrobertson Yes, thanks for getting the review fixed!

I incorporated your feedback, but also updated a method that has been deprecated for a long time to use what I think is the current approach. I'm not sure about OSANull for the storage options, but it seems to be working for me.

Could you and @skurfer run this locally to make sure your applescripts are also still working as expected?

[pjrobertson]

Code looks good to me.

By 'deprecated method' you mean:

[[OSAScript alloc] initWithScriptDataDescriptor:aed
            fromURL:[NSURL fileURLWithPath:scriptPath]
            languageInstance:[language sharedLanguageInstance]
            usingStorageOptions:OSANull error:&err];

Not deprecated for me on 10.14 ;-) AVAILABLE_MAC_OS_X_VERSION_10_6_AND_LATER :D

I'll run with this now

[pjrobertson]

Tested, LGTM. I'll leave open in case @skurfer has anything to add :)

[pjrobertson]

I'm going to merge this now, so we can get it out there :)