Re: [ruby-gnome2-devel-ja] [patch] GtkBindingSet

[Masao M. <mu...@hi...>]

むとうです。

小宮さん、はじめまして。

激しく疲れてるのでコメントだけです。
もしよろしければコメント吸収版のパッチを
もう一度投げていただけないでしょうか。
＃すみません、ちと風邪気味＆デスマーチ気味で....。

On Fri, 19 Dec 2003 00:48:09 +0900 (JST)
Kenichi Komiya <ko...@ma...> wrote:

> はじめまして、小宮と申します。
> 
> GtkBindingSet#add_signal を動くようにしてみました。gtkbindings.h 
> の他の publich な API も動作をひととおり確認しています。

それは助かります。
 
> 追加メソッド
> 
>   * Gtk::BindingSet.new
>     Gtk::BindingSet#initialize をうまく実装できなかったので(※)
>     new をオーバーライドしました。
（順番入れ替えてます）
> ※ initialize をうまく実装できなかったというのは、始め initialize
>    の中で G_INITIALIZE(self, binding_set) のようにしていたのです
>    が、binding_set のコピーがラップされてしまってうまく動作しな
>    かったという事です。私が Ruby/Gtk の C API をまだ把握していな
>    いため正しいやり方を見落としているだけかもしれません。

なるほど。
ただ、newをオーバーライドするとGtk::BindingSetのサブクラスを作るのが
難しくなってしまうので、できればinitializeのままで行きたいところですね。

そこで、申し訳ないのですが、以下の２つを確認してみていただけないで
しょうか。

(案１)
rbgobj_boxed_not_copy_obj(GTK_TYPE_BINDING_SET);
をInit_gtk_bindings()の先頭あたりで宣言する。

(案２)
gtk_bindingset_copy(const GtkBindingSet* bin)の方をいじる。

static GtkBindingSet*
gtk_bindingset_copy(const GtkBindingSet* bin)
{
  /* not copy here */
  return bin;
}

一応、案１＞案２の優先度です。どちらも上記以外は元(initialize)のままです。

＃どちらも、逆にコピーが必要そうな場所があったら明示的に
＃g_newしないといけない場所が出てくるかもしれません。
 
>   * Gtk::BindingSet.find
>     処理内容を考慮してクラスメソッドに変更しました。

すみません、これ、バグでした....。
 
>   * Gtk::BindingSet.by_class
>     gtk_binding_set_by_class()。 Gtk::Object.binding_set の方が
>     良いでしょうか？

確かにそうですね。
しかも、使用目的を考えると、引数を省略可能にして以下のようにも
書けたりすると良さげですがどうでしょう。
＃書けるのかな(^^;)。

class MyTreeView < Gtk::TreeView
  bset = binding_set
  bset.add_signal(Gdk::Keyval::GDK_space, 0, "move_cursor", 
                  Gtk::MOVEMENT_PAGES, 1, false)

  def initialize
     :
  end
end

>   * Gtk::BindingSet.activate
>     gtk_bindings_activate()。Gtk::Object#activate_binding とか
>     の方が良いでしょうか？

Gtk::Object#bindings_activateというのがあるのでそちらを使うと
よろしいかと。

>   * Gtk::BindingSet#add_signal
>     gtk_binding_entry_add_signal()。
> 
>     シグナルに渡せるパラメータは Float, 整数、文字列のみ。to_int 
>     と to_str の順で変換を試みる。true/false は 1/0 に変換される。

> +            arg->d.string_data = StringValueCStr(param);

は念のためRVAL2CSTRを使ってください。

> +        } else if (rb_respond_to (param, rb_intern("to_str"))) {
> +        } else if (rb_respond_to (param, rb_intern("to_int"))) {

は微妙ですねぇ(^^;)。
to_intの方は、GLib::EnumかGLib::FlagsかInteger型の時、って限定
した方が良さそうな気がしますがどうでしょうか。
to_strの方は、極端な話to_sにしちゃってif文の優先度を下げるようにするとか。

true/falseのところはGTK+のソースを読むと、G_TYPE_LONGではなく
G_TYPE_BOOLEANを明示的に使っているところが多いのですが、
こちらもそうした方が良いのではないでしょうか。

> 削除メソッド
> 
>   * Gtk::BindingSet#entry_remove
>     gtkbindings.h に public ではないと書かれていたので消しました。

おぉ、気づきませんでした。ありがとうございます。
でも、APIドキュメントにはあるんですねぇ、自動生成したからかなぁ。

あと、サンプルコードの方も頂いちゃいたいのですがよろしいでしょうか？

--- 
.:% Masao Mutoh<mu...@hi...>