[libcaca] Re: Ruby binding

[Sam Hocevar <sam@zoy.org>]

On Mon, Nov 12, 2007, Pascal Terjan wrote:

> as I was frustrated when I read in TODO that Ruby bindings are "Not that
> important", I started some.
> 
> I currently only have the canvas part of libcucul done (meaning all
> functions that have a cucul_canvas_t* as first argument except
> cucul_dither_bitmap).

   Excellent. As I said on IRC, they look OK for trunk quality, you can
check them in now that you have SVN access :)

   I have a few comments/questions though.

> irb(main):003:0> Cucul::Canvas.instance_methods.sort -
> Cucul::Canvas.ancestors[1].instance_methods
> => ["attr=", "blit", "clear", "create_frame", "cursor_x", "cursor_y",
> "draw_box", "draw_circle", "draw_cp437_box", "draw_ellipse",
> "draw_line", "draw_polyline", "draw_thin_box", "draw_thin_ellipse",
> "draw_thin_line", "draw_thin_polyline", "draw_thin_triangle",
> "draw_triangle", "export_memory", "fill_box", "fill_ellipse",
> "fill_triangle", "flip", "flop", "frame=", "frame_count", "frame_name",
> "frame_name=", "free_frame", "get_attr", "get_char", "get_cursor_x",
> "get_cursor_y", "get_frame_count", "get_frame_name", "get_handle_x",
> "get_handle_y", "get_height", "get_width", "gotoxy", "handle_x",
> "handle_y", "height", "height=", "import_file", "import_memory",
> "invert", "printf", "put_attr", "put_char", "put_str", "rotate_180",
> "rotate_left", "rotate_right", "set_attr", "set_boundaries",
> "set_color_ansi", "set_color_argb", "set_frame", "set_frame_name",
> "set_handle", "set_height", "set_size", "set_width", "stretch_left",
> "stretch_right", "width", "width="]

   Is it the Ruby way to have get_width, set_width, width and width= at
the same time? It looks to me like Perl "let's confuse the user in 53
different ways" nonsense.

> +static VALUE put_char(VALUE self, VALUE x, VALUE y, VALUE ch)
> +{
> +    cucul_put_char(_SELF, NUM2INT(x), NUM2INT(y), NUM2ULONG(ch));
> +    return self;
> +}

   Doesn't look like very strong typing. Is there a way to tell Ruby ch
is an Unicode character? Or does it handle them just like numbers?

> +static VALUE draw_polyline(VALUE self, VALUE x, VALUE y, VALUE ch)

   I suggest using one single array of coordinate tuples here. In fact,
I suggest using coordinate tuples everywhere instead of two x/y values.

> +    rb_define_const(mCucul, "BLACK", INT2FIX(0x00));

   Please use something like this here:

#define CUCUL_CONST(x) \
   rb_define_const(mCucul, #x, INT2FIX(CUCUL_##x))

> Index: ruby/Makefile

   A distclean target would be nice here, but I'll autotoolify the lot
anyway, so don't bother.

-- 
Sam.
-- 
This is the libcaca mailing-list, see http://libcaca.zoy.org/
Trouble unsubscribing? Please contact <sam@zoy.org>
List archives are at http://libcaca.zoy.org/list/