jrubyfx icon indicating copy to clipboard operation
jrubyfx copied to clipboard

Nesting of method_missing types in generated precompiled.rb can erroneously call add

Open enebo opened this issue 9 years ago • 5 comments

I add support for add for TabPane in exts.yml:

"Java::JavafxSceneControl::TabPane":
   logical_children: tabs
   method_missing: "Java::JavafxSceneControl::Tab"
   add: get_tabs
``
Then if I call a program like this:

```ruby
#!/usr/bin/env jruby
require 'jrubyfx'

class TabPaneApp < JRubyFX::Application
  def start(stage)
    with(stage, title: 'Tabs', width: 400, height: 250) do
      layout_scene do
        border_pane do
          pane = tab_pane do
            5.times do |i|
              text = "tab#{i}"
              tab(text: text, content: hbox {label(text)})
            end
          end
          center pane
        end
      end
    end.show
  end
end

TabPaneApp.launch

I get a crash. The reason is hbox calls the method_missing for TabPane...but...TabPane extends parent which will call 'add' because the hbox is a Node. Clearly add implementation only wants types it believes should be added and it should ignore any adds from parent types. At the moment I think the only solution I can think of is to add an extra if check to the specific add method of all types:

  def add(value)
    get_tabs() << value if value.kind_of?(Java::JavafxSceneControl::Tab)
  end

I find adding this mildly weird but we do not want random less specific types getting added. Another possibly solution is to pass the acceptable type in as an argument to type to at least make it more explicit?

enebo avatar Nov 24 '15 23:11 enebo

I just realized .yml file only provides type to method_missing attribute and not to add attribute. This is more complicated in that it will require a new format.

enebo avatar Nov 25 '15 00:11 enebo

Yea, I special cased stuff manually and let exts.yaml do the stuff that multiple controls has the same implementation of

byteit101 avatar Nov 25 '15 00:11 byteit101

@byteit101 ok it makes sense that this can be manual but I have to think the scenario I put above must be very easy t trip over for anything more specific than Node.

enebo avatar Nov 25 '15 00:11 enebo

@byteit101 Actually I guess not. All of these seem to be extending Object or the same thing as their parents. With some change we could probably make this fit a lot more cases without needing manual core_ext entries.

enebo avatar Nov 25 '15 00:11 enebo

Thinking about this a little more over dinner....we could add an attribute to xml called: add_type: with a string list of a valid type (this could be a list perhaps but let's not go crazy). We can then remove the type from method_missing. The code which would be generated would be:

  def valid_add_type?(value)
    value.kind_of?
  end

  def add(value)
    get_tabs() << value
  end

I think then this means all generated method_missing could disappear so long as all paths ended up hitting:

  def method_missing(name, *args, &block)
    super(name, *args, &block).tap do |obj|
      add(obj) if valid_add_type?(obj) && !name.to_s.end_with?('!')
    end
  end

For this pattern to work well we should make sure all method_missing which are left (for manual entries) end up calling super to get to this logic. We could also generate all the method missings which are done now as well and that would not be an issue but it seems much less necceary having a type add predicate instead. This definitely will solve the described problem...Improvement?

enebo avatar Nov 25 '15 00:11 enebo