SketchUp / api-issue-tracker

Public issue tracker for the SketchUp and LayOut's APIs
https://developer.sketchup.com/
38 stars 10 forks source link

REQ: Sketchup::Menu #get_submenu, #has_submenu?, #has_item? methods #423

Open DanRathbun opened 4 years ago

DanRathbun commented 4 years ago

SketchUp Ruby API Feature Request


Sketchup::Menu#get_submenu Getter Method

Please implement a Sketchup::Menu#get_submenu method so that authors can have separate extensions share the same submenu.

The method should likely return nil if the submenu does not yet exist.

The method might have a :create boolean named argument (defaulting to false) that will create the submenu if it does not yet exist.


Sketchup::Menu#has_submenu? Query Method

Returns boolean if the menu has a submenu named per the argument.


Sketchup::Menu#has_item? Query Method

Returns boolean if the menu has a menu item named per the argument or matching the integer ID given.


Getter Method Implementation

Currently, each call to Sketchup::Menu#add_submenu creates a duplicate submenu object. For example, at the console ...

plugins_menu = UI.menu("Plugins")
submenu = plugins_menu.add_submenu("Test")
submenu.add_item("Hello World") {
  UI.messagebox("Hi there!")
}
submenu = plugins_menu.add_submenu("Test")
submenu.add_item("Hello World 2") {
  UI.messagebox("Hi there once again!")
}

... creates 2 submenus, both named "Test".

QUESTION: Should we consider changing the behavior of the #add_submenu method, so that it only creates a submenu of the string argument if it does not yet exist, and otherwise returns the existing submenu ?

If you prefer that #add_submenu is changed, then our usage would be more like ...

main_menu =  UI.menu("Plugins")
my_menu = main_menu.add_submenu("Author")
if my_menu
  # add menu items or nested submenus
end

My thoughts on use for a new #get_submenu method are this ...

main_menu =  UI.menu("Plugins")
my_menu = main_menu.get_submenu("Author")
my_menu = main_menu.add_submenu("Author") if my_menu.nil?

BUT, if the new method also had a create argument, then we could simply do this ...

main_menu =  UI.menu("Plugins")
my_menu = main_menu.get_submenu("Author", create: true)

Boolean Query Methods

Along with a getter, it would be nifty if the Sketchup::Menu class had existence boolean query methods #has_submenu?() and #has_item?().

Example usage ...

main_menu =  UI.menu("Plugins")
if main_menu.has_submenu?("Author")
  my_menu = main_menu.get_submenu("Author")
else
  my_menu = main_menu.add_submenu("Author")
end
unless my_menu.has_item?(menu_text) # could also take an ID
  my_menu.add_item(menu_text) { call_some_function() }
end

?

Eneroth3 commented 4 years ago

Makes sense to me. The only thing I'd like to change is removing the get and has prefix as these generally aren't encouraged in Ruby. They make sense in e.g. C but with the question mark and parameter it is quite clear what the method means in Ruby anyway. Also it is more consistent to UI.menu.

DanRathbun commented 4 years ago

The only thing I'd like to change is removing the get and has prefix as these generally aren't encouraged in Ruby.

I have to disagree. A menu is a collection. Ruby core collections have "has_...?" boolean methods. It is convention.

Ie, the "item" is not a property (instance attribute) of the menu object, so has_item? is appropriate because it asks the collection object if it has the item. Same hold true for has_submenu?

If we were instead asking the menu if it were empty, then that is a property of the collection itself, and deserves a plain ol' empty? method.

The "get_submenu" is following the convention of "add_submenu" which is a factory method, which follows the naming convention SketchUp's API set with it Entities collection factory methods.

Ordinarily from a collection you'd want to have a [name] method that gets member object or returns nil, but in this case a menu collection can have a mixed set of members. Now, I wouldn't scream and yell if Thomas decided to implement as [name] as it could also return item objects (or actually the UI::Command attached to it.)

?

DanRathbun commented 4 years ago

Modifying my previous statements ... I was doing some work last night and examining the core Hash class.

I believe that API collection classes accessible by key (name, ID, etc.) should mimic (if not be a subclass of) the core Hash class.

So, Hash basic query methods are: #has_key?, #include?, #key?, and #member? (the latter 3 are all aliases of #has_key?.)

Anyway, ... if the convention is followed from class Hash, we'll both be happy.