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

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

Open
enebo opened this issue Nov 24, 2015 · 5 comments

Comments

@enebo
Copy link
Member

enebo commented Nov 24, 2015

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
Copy link
Member Author

enebo commented Nov 25, 2015

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.

@byteit101
Copy link
Member

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

@enebo
Copy link
Member Author

enebo commented Nov 25, 2015

@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
Copy link
Member Author

enebo commented Nov 25, 2015

@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
Copy link
Member Author

enebo commented Nov 25, 2015

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?

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