Re: libvtemm review for module proposal

[Krzesimir Nowak <qdlacz gmail com>]

On Mon, 2009-08-10 at 19:28 +0200, Krzesimir Nowak wrote:
> 1. API review - since I am not experienced designer, I would very much
> appreciate some insight and comments with regard to API (method
> prototypes need to be changed? remove some classes which seems to be
> redundant, useless, overly complicated?). Since libvtemm is not widely
> used, breaking things (again) does not pose a problem.

I'll start with some doubts I have:


1. void vte_terminal_im_append_menuitems(VteTerminal*, GtkMenuShell*)

I wrapped it as:
void Terminal::im_append_menuitems(Gtk::MenuShell&)
but I have no clue if this is proper. It calls
gtk_im_multicontext_append_menuitems, which seems to be not wrapped
(along with GtkIMContext, GtkIMContextSimple and GtkIMMultiContext) and
it modifies given MenuShell.


2. void vte_terminal_get_cursor_position(VteTerminal*, glong*, glong*),
void vte_terminal_get_padding(VteTerminal*, int*, int*),
char* vte_terminal_match_check(VteTerminal*, glong, glong, int*)

Here I decided to move output parameters out from parameter list and put
them in separate classes:
CursorPosition Terminal::get_cursor_position(),
Padding Terminal::get_padding(),
Match Terminal::match_check(long, long)

I did it to separate input from output and have a code easier to read
(IMHO), but to do this I had to create some new small classes. Is it too
much of them? Is it better just to put references to output variables in
parameter list? (void Terminal::get_padding(int&, int&) )


3. GtkAdjustment* vte_terminal_get_adjustment(VteTerminal*)

I wrapped it as:
Gtk::Adjustment& Terminal::get_adjustment()
Is it better to return a reference or a pointer? I decided to return a
reference, because now it can be nicely used for Gtk::(H|V)Scrollbar
like this:
Vte::Terminal terminal;
Gtk::VScrollbar my_scrollbar(terminal.get_adjustment());
That is typical and most common use of get_adjustment method.


4. VteTerminalAccessible

It doesn't have any methods on its own - it implements some interfaces.
I hope it is wrapped properly. (terminalaccessible.hg)


5. _vte_pty_open, _vte_pty_close, _vte_pty_get_size, _vte_pty_set_size,
_vte_pty_set_utf8, _vte_pty_get_pty

At first, I just put them in Vte::Pty namespace and wrapped them to use
C++ types. But later I removed the namespace and put them to separate
class, since they operate mostly on a sort of descriptor, which happens
to be `int', so this class keeps a descriptor and a bool value
indicating ownership. I was wondering if it is not too complicated. But
then, these functions probably aren't widely used and are not meant to,
as there are no documentation for them, only public header file.


I'd very much appreciate your comments,
Krzesimir Nowak