add keyword arg support to callMethod directly#9403
Conversation
|
I like this and I also like the PR only pushes this into embedding API itself. We have a long running task to remove callInfo and to make kwargs more first class and not based on some out-of-band field set per thread. My only thing I wonder about is that Map<String, Object> would somehow be nicer if it could be String or RubySymbol for people who already know they are directly interacting with Ruby from an embedded context. Not sure I am recommending another set of overloads here or whether that case is solved by our larger effort to rewrite kwargs but it seems like a reasonable use case (also it probably could be added as a later PR). |
|
Yes, this seems like a reasonable addition. @enebo To your point, we could use Map<CharSequence, Object> since RubySymbol and RubyString also implement CharSequence. Dunno if it's worth it or not. I'll review. |
headius
left a comment
headius
left a comment
There was a problem hiding this comment.
Generally on the right track. I think it's worth discussing whether it's safer/cleaner to just add new method names for the kwargs calls rather than more complicated overloads that might be ambiguous in some cases.
…port_to_call_method
…ub.com/drzaiusx11/jruby into add_keyword_arg_support_to_call_method # Conflicts: # core/src/main/java/org/jruby/embed/EmbedRubyObjectAdapter.java # core/src/main/java/org/jruby/embed/ScriptingContainer.java # core/src/main/java/org/jruby/embed/internal/EmbedRubyObjectAdapterImpl.java # core/src/test/java/org/jruby/embed/ScriptingContainerTest.java
|
ok pushed the updates
I'm also purposefully avoiding using 'Object...' for now in the new APIs, opting for arrays to avoid the inevitable disambiguation issues discussed. We can add N-arg positional overloads for N...4 or something sane if thats more ergonomic but I think arrays isn't a terrible first pass |
3 participants
started a discussion here: #9402, copying verbatim here for reference:
I've been using JRuby to wrap some pre-existing Gems to be called by Java using the ScriptingContainer interface provided by JRuby 10.x, however, whenever a Gem uses keyword arguments there doesn't seem to be a clean way to force a Map to be treated as the keyword arguments for that Ruby API. I see theres a ThreadContext.CALL_KEYWORD flag I can use, but it seems more an internal implementation detail than something that should be exposed to scripting users. Afaict this works:
What I'm proposing is adding further callMethod method overloads to include a last argument of Map<String, Object> or similar that will become those keyword arguments (convert strings to symbols automatically, call convertJavaToUsableRubyObject() on values, etc. My guess on why this hasn't come up before is that any scripting use cases predated Ruby's 3.2 order argument changes around auto-conversion of the last hash into keyword args. Since 3.2+ that behavior is disallowed so some other mechanism needs to exist going forward. I think what I propose makes sense here, but i could be missing something important.
This is my attempt at implementing it