wxruby development - Mar 2008 - Proposal for an improved API for Sizer (and ToolBar)

Hi all,

This is a proposal for supporting keyword arguments to Sizer#add
following the thread below :
http://rubyforge.org/pipermail/wxruby-development/2008-March/001244.html

After a deeper look at lib/wx/keyword_ctors.rb and
lib/wx/keyword_defs.rb, I understood the whole thing :-).
I just reused and modified slightly the method args_as_list.
The new Sizer#add_item combines the features of add and insert in
addition to the support of keyword arguments.

Let me know what do you think about this proposal as this can be
applicable to ToolBar#add_tool as well.


So here is the proposal with a simple test application :

require ''wx''

module Wx

  # Return the real arguments from the specified arguments definition and
  # the supplied mixed arguments

  # args_def : the ordered list of arguments with default value
  # e.g. [ [:proportion, 0], [:flag, 0], [:border, 0] ]

  def self.args_as_list(args_def, *mixed_args)
    # get keyword arguments from mixed_args if supplied, else empty
    kwa = mixed_args.last.kind_of?(Hash) ? mixed_args.pop : {}
    out_args = []
    args_def.each_with_index do | (arg_name, arg_value), i |
      if arg = mixed_args[i] # use the supplied list arg
        out_args << arg
      elsif kwa.key?(arg_name) # use the keyword arg
        out_args << kwa[arg_name]
      else # use the default argument
        out_args << arg_value
      end
    end
    out_args
  rescue
    Kernel.raise ArgumentError, "Bad arg composition of
#{mixed_args.inspect}"
  end

end


class Wx::Sizer

  # Add or insert an item (window, child sizer or spacer) to the sizer.
  # A spacer is defined as an array containing the width and the height.
  # Other arguments are optional and may be specified by position or via
  # keyword.
  # If index = -1, the item is added at the end. Otherwise, the item is
  # inserted at index.
  # Equivalent signature for positional arguments :
  # add_item(item, index = -1, proportion = 0, flag = 0, border = 0,
user_data = nil)
  #
  # (add and insert are kept for backward compatibility but should be
deprecated)
  def add_item(item, *opt_args)

    full_args = []

    # extract the width and the height in the case of a spacer addition
    if item.kind_of?(Array)
      Kernel.raise ArgumentError,
        "Invalid Sizer specification : [width, height] expected" if
element.size != 2
      full_args << item[0] << item[1]
    else
      full_args << item
    end

    # the ordered list of optional arguments definition
    args_def = [ [:index, -1], [:proportion, 0], [:flag, 0], [:border,
0], [:user_data, nil] ]

    args = Wx::args_as_list(args_def, *opt_args)

    # retrieve the index
    index = args.shift
    # update the full arguments list with the optional arguments (except index)
    full_args.concat(args)

    if index == -1
      # default index => call the original add method
      add(*full_args)
    else
      # index is specified => call the original insert method
      full_args.unshift(index)
      insert(*full_args)
    end

  end

end


# Simple test
Wx::App.run do

  frame = Wx::Frame.new(nil, :title => ''Sizer API
improvements'') do

    sizer = Wx::BoxSizer.new(Wx::VERTICAL)

    button1 = Wx::Button.new(self, :label => ''Button 1'')
    button2 = Wx::Button.new(self, :label => ''Button 2'')
    button3 = Wx::Button.new(self, :label => ''Button 3'')

    # use of positional arguments
    sizer.add_item(button1, -1, 1, Wx::EXPAND)
    # use of keyword arguments without index
    sizer.add_item(button3, :proportion => 1, :flag => Wx::EXPAND)
    # use of keyword arguments with index specified
    sizer.add_item(button2, :index => 1, :proportion => 1, :flag =>
Wx::EXPAND)

    self.sizer = sizer

  end

  frame.show

end


Cheers.

Chauk-Mean.

Hi

Chauk-Mean P wrote:
> This is a proposal for supporting keyword arguments to Sizer#add
> following the thread below :
> http://rubyforge.org/pipermail/wxruby-development/2008-March/001244.html
>
> After a deeper look at lib/wx/keyword_ctors.rb and
> lib/wx/keyword_defs.rb, I understood the whole thing :-).
> I just reused and modified slightly the method args_as_list.
>   
Thanks for following this up. This looks like a good implementation of 
args_as_list. One question, I guess, is whether it''s better to create a
new keyword-args version, or whether, as with the constructors, to try 
and support keyword and positional args simultaneously.
> The new Sizer#add_item combines the features of add and insert in
> addition to the support of keyword arguments.
>   
Looks good, but my main problem with Sizer#add in the C++ API is not so 
much the absence of keyword arguments as the confusion of functions 
within the arguments. Sizers control sizing, alignment and borders. 
Sizing is controlled partly by the first and partly by the second, 
alignment by the second, and borders partly by the second and partly by 
the third. So what I tried to do with layout.rb was to split these out:

add(child, :proportion => 1, :expand => 1, :align => Wx::ALIGN_CENTRE, 
:border_size => 5, :border_pos => Wx::TOP)

This needs a custom method, but it''s mostly written already in wxSugar.
What do you tink?
> Let me know what do you think about this proposal as this can be
> applicable to ToolBar#add_tool as well.
I think ToolBar#add is less confused than Sizer#add, so looks good to me.

cheers
alex

Hi Alex,

2008/3/19, Alex Fenton <alex at pressure.to>:
> Thanks for following this up. This looks like a good implementation of
> args_as_list. One question, I guess, is whether it''s better to
create a
> new keyword-args version, or whether, as with the constructors, to try
> and support keyword and positional args simultaneously.
With my proposal, keyword and positional args are also supported
simultaneously.
This is indicated in the comment of the Sizer#add_item method about
the equivalent signature for positional arguments.
This can also be seen in the first line of "sizer.add_item" in the
example application.

The reason why I modified the original version of args_as_list is that
that method relies on an instance variable dedicated to the
constructor. I think that keyword arguments can be useful for various
methods of a given class. Thus, taking the definition of an ordered
list of arguments as a parameter seems to be more flexible.

I agree that it is not good to have 2 methods that are very similar.
If you like the modified args_as_list method, I think that
keyword_ctors.rb and keyword_defs.rb can be updated to use this
modified version.
> > The new Sizer#add_item combines the features of add and insert in
> > addition to the support of keyword arguments.
> >
> Looks good, but my main problem with Sizer#add in the C++ API is not so
> much the absence of keyword arguments as the confusion of functions
> within the arguments. Sizers control sizing, alignment and borders.
> Sizing is controlled partly by the first and partly by the second,
> alignment by the second, and borders partly by the second and partly by
> the third. So what I tried to do with layout.rb was to split these out:
>
> add(child, :proportion => 1, :expand => 1, :align =>
Wx::ALIGN_CENTRE,
> :border_size => 5, :border_pos => Wx::TOP)
>
> This needs a custom method, but it''s mostly written already in
wxSugar.
> What do you tink?
Well, I haven''t thought about that.
But this looks like an alternative way for laying out things.
> > Let me know what do you think about this proposal as this can be
> > applicable to ToolBar#add_tool as well.
> I think ToolBar#add is less confused than Sizer#add, so looks good to me.
I will post a proposal for ToolBar tomorrow morning.

Cheers.

Chauk-Mean.

Hi,

2008/3/19, Chauk-Mean P <chauk.mean at gmail.com>:
> > > Let me know what do you think about this proposal as this can be
> > > applicable to ToolBar#add_tool as well.
> > I think ToolBar#add is less confused than Sizer#add, so looks good to
me.
>
> I will post a proposal for ToolBar tomorrow morning.
First, I improved further the args_as_list method by checking that the
keyword arguments supplied are valid ones.

Regarding the new method ToolBar#add_item :
- this method adds or inserts an item (tool, check tool, radio tool)
to the toolbar and returns the ID of this item. The ID is no more
mandatory.
The position of the item is not returned but it may be retrieved
easily via find_by_id if needed.
- it supports keyword arguments and positional arguments
- the only mandatory argument is the bitmap.
Indeed, by default a toolbar displays only icons. So the label should
not be mandatory.

You can find attached the full source code with :
- the revised implementation of the method args_as_list,
- the implementation of the new Sizer#add_item and ToolBar#add_item methods
- an example application that makes use of these new methods.

I''d really like to see such features included in wxRuby so feel free
to modify this code.

Cheers.

Chauk-Mean.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sizer_toolbar_redef.rb
Url:
http://rubyforge.org/pipermail/wxruby-development/attachments/20080320/09bf1e48/attachment.pl

Hi all,

2008/3/20, Chauk-Mean P <chauk.mean at gmail.com>:
> I''d really like to see such features included in wxRuby so feel
free
> to modify this code.
What about my proposals ?
Is there any intention to integrate them in wxRuby ?

Cheers.

Chauk-Mean.

Chauk-Mean P wrote:
>> I''d really like to see such features included in wxRuby so
feel free
>> to modify this code.
>>     
>
> What about my proposals ?
> Is there any intention to integrate them in wxRuby ?
>   
Sorry, I''ve been meaning to get around to this. My plan is to integrate
your work on Toolbar largely as is for the next release. I just wonder 
whether you think we might add it as mixed-args like the constructors?

For Sizer I plan to merge it with the stuff in wxSugar''s layout.rb that
separates out the different aspects of add like border/sizing/proportion

thanks
alex

Alex,

2008/3/28, Alex Fenton <alex at pressure.to>:
> Chauk-Mean P wrote:
> >> I''d really like to see such features included in wxRuby
so feel free
> >> to modify this code.
> >
> > What about my proposals ?
> > Is there any intention to integrate them in wxRuby ?
> >
>
> Sorry, I''ve been meaning to get around to this. My plan is to
integrate
> your work on Toolbar largely as is for the next release.
That''s great.
> I just wonder whether you think we might add it as mixed-args
> like the constructors?
I''m not sure to understand what you mean.
mixed-args support  = positional args support + keyword args support ?
If yes, my proposals already support them.
> For Sizer I plan to merge it with the stuff in wxSugar''s layout.rb
that
> separates out the different aspects of add like border/sizing/proportion
Cool !

Cheers.

Chauk-Mean.

Chauk-Mean P wrote:
> I''m not sure to understand what you mean.
> mixed-args support  = positional args support + keyword args support ?
> If yes, my proposals already support them.
>   
Yes, sorry, you''re right. All looks good then!

alex

Hi Chauk-Mean

Chauk-Mean P wrote:
> 2008/3/19, Chauk-Mean P <chauk.mean at gmail.com>:
>   
>> I will post a proposal for ToolBar tomorrow morning.
>>     
>
> First, I improved further the args_as_list method by checking that the
> keyword arguments supplied are valid ones.
>
> Regarding the new method ToolBar#add_item :
Just to mention that I added the ToolBarTool class to wxRuby so it''s 
consistent with MenuItem and SizerItem. It makes the API easier in that 
you can say:

tool = tool_bar.add_tool(...)
evt_tool tool, :on_tool_action

I think the bulk of your proposal still stands, except no generate_id 
method is needed, and I think it should now return the ToolBarTool 
object. So I will still look to merge for the next release.

cheers
alex

alex

Hi Alex,

2008/4/18, Alex Fenton <alex at pressure.to>:
> Just to mention that I added the ToolBarTool class to wxRuby so
it''s
> consistent with MenuItem and SizerItem.
>
> I think the bulk of your proposal still stands, except no generate_id
> method is needed, and I think it should now return the ToolBarTool
> object. So I will still look to merge for the next release.
That''s fine. I''m looking forward the next release.

Cheers,

Chauk-Mean.

Chauk-Mean P wrote:
>> I think the bulk of your proposal still stands, except no generate_id
>> method is needed, and I think it should now return the ToolBarTool
>> object. So I will still look to merge for the next release.
>>     
>
> That''s fine. I''m looking forward the next release.
Cool - I meant to say, could you also submit a patch to fix the 
documentation please? These kind of enhancements are little use unless 
they are in the API docs.

thanks
alex

2008/4/21, Alex Fenton <alex at pressure.to>:
> > That''s fine. I''m looking forward the next release.
> Cool - I meant to say, could you also submit a patch to fix the
> documentation please? These kind of enhancements are little use unless
> they are in the API docs.
Alex,

I can do it but I will not be available for the next 2 weeks for family reasons.
If wxRuby 1.9.6 is not expected to be released during these 2 weeks,
then it is OK for me.

Chauk-Mean.

Chauk-Mean P wrote:
> I can do it but I will not be available for the next 2 weeks for family
reasons.
> If wxRuby 1.9.6 is not expected to be released during these 2 weeks,
> then it is OK for me.
No problems - given that ruby changes have introduced a big crasher bug 
with the latest stable version 1.8.6-114, we may release a little bit 
earlier. I would be very happy to integerate your patch at any time 
afterwards though, anyway.

best
alex

Hi Chauk-Mean

Chauk-Mean P wrote:
>>>> Let me know what do you think about this proposal as this can
be
>>>> applicable to ToolBar#add_tool as well.
>>>>         
>>> I think ToolBar#add is less confused than Sizer#add, so looks good
to me.
>>>       
>> I will post a proposal for ToolBar tomorrow morning.
>>     
>
> First, I improved further the args_as_list method by checking that the
> keyword arguments supplied are valid ones.
>
> Regarding the new method ToolBar#add_item
I have just merged this in, with some minor modifications so it''s
better
integrated with existing code. Thanks again for the patch. Sorry it took 
me a while to get round to it.

cheers
alex

Hi (again) Alex,
> I have just merged this in, with some minor modifications so it''s
better
> integrated with existing code. Thanks again for the patch.
You''re welcome.
> Sorry it took me a while to get round to it.
You''re forgiven :-). You''ve done an incredible work with the
support
of Ruby 1.8.7 and 1.9.

Cheers.

Chauk-Mean.