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

Better mechanism for non-DSL calls using converters (or make converter_for do more) #18

Open
enebo opened this issue Jan 31, 2013 · 6 comments

Comments

@enebo
Copy link
Member

enebo commented Jan 31, 2013

I just hacked this for drag_event.rb:

  tmc = enum_converter(Java::javafx::scene::input::TransferMode)
  converter_for :accept_transfer_modes, &tmc

  # FIXME: For non-dsl calls like this we want converter logic
  alias :accept_transfer_modes_orig :accept_transfer_modes
  def accept_transfer_modes(*values)
    accept_transfer_modes_orig *attempt_conversion(self, "accept_transfer_modes", *values)
  end

This snippet shows how I worked around being able to:

        event.accept_transfer_modes :move

Notice since we are just calling a method on an object our DSL is not firing the converters on the arguments. I was going to just make another method for this but I am wondering whether converter_for should do this logic by default. What do we think?

@byteit101
Copy link
Member

ah, this looks like a missing enum conversion unless I am mis-reading this?
wait, why is it not firing the converters? converter_for should have this in it

@enebo
Copy link
Member Author

enebo commented Feb 1, 2013

converter_for just makes the methods for converting and the DSL mechanics end up using the converter method. So I think since this is a not a fcall we end up not calling the conversion logic.

I am thinking about re-thinking how we specify converters and maybe making it a generic gem. One issue I realized is that arity is specified based on how we provide arg lists in Ruby versus arity of Java method signatures we want to map to. Converting a method which takes one int and then a varargs should be specifiable, but the syntax we are using cannot help us.

@byteit101
Copy link
Member

Now that we have the declarative yaml file, i've been making it generate normal snake cased converters instead of DSL only converters and I think we should phase out dsl-only converters as that is a breach of expectation

@enebo
Copy link
Member Author

enebo commented May 8, 2013

When I originally made them DSL-only it was with the idea that people may still want access to the actual method without our magic in the way. I can see your meeting expectations argument but I figured it was less of a reality issue since very few Rubyists use the snake-cased syntax.

To play devil's advocate with my original change I will say that the converters do pass through correct values if they are already in the proper type. Or perhaps another way of stating that is if I pass in a Color instance our converters won't change it.

Perhaps based on my last argument the only reason someone would want the original proxy method is to avoid performing a block dispatch. It would be for purely a fine-tuning if block dispatch overhead is killing their app. I don't know if that is a good enough scenario to not just go with what you suggest. Worst-case we can change generator to make aliases so we can still call the original method later if it becomes an issue.

@byteit101
Copy link
Member

very few Rubyists use the snake-cased syntax.

??????? me is confused??????

To play devil's advocate with my original change I will say that the converters do pass through correct values if they are already in the proper type. Or perhaps another way of stating that is if I pass in a Color instance our converters won't change it.

yea. I wasn't suggesting removing that.

Perhaps based on my last argument the only reason someone would want the original proxy method is to avoid performing a block dispatch.

cantTheyThenUseCamels?

My suggestion is:
move remaining converters to yml file if possible
converters expand to override the ruby-like snake cased methods
use same converter logic as before where we only convert stuff that is needing converting

clone master and run rake reflect and then stare at jrubyfx/core_ext/precompiled.rb and the second half of jrubyfx/dsl.rb

also, I drafted this up: https://github.com/jruby/jrubyfx/wiki/Developer-overview thoughts/suggestions?

@enebo
Copy link
Member Author

enebo commented May 8, 2013

Sorry I meant CamelCased not snake_cased :)

I was just playing devil's advocate in that second paragraph...not putting any words in your mouth.

Actually I will come on irc since I think I totally missed what you want to do. I thought you wanted both snake and camel-cased methods to use converters. I definitely misread this but now I don't know what you mean...

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

No branches or pull requests

2 participants