Re: [PATCH] Support Copy/Paste/Reset on target terminal

[Dodji Seketeli <dodji seketeli org>]

Hello Fabien,

Sorry for the delay in replying to this.  For some reason this message
just felt below my radar.

Fabien Parent <parent f gmail com> a écrit:

> Hello Dodji,
>
>>> diff --git a/menus/terminalmenu.xml b/menus/terminalmenu.xml
>>> new file mode 100644
>>> index 0000000..78911e8
>>> --- /dev/null
>>> +++ b/menus/terminalmenu.xml
>>> @@ -0,0 +1,10 @@
>>> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
>>> + <ui>
>>> +  <popup name='TerminalMenu'>
>>> +    <menuitem action='CopyAction'/>
>>> +    <menuitem action='PasteAction'/>
>>> +    <separator/>
>>> +    <menuitem action='ResetAction'/>
>>> +  </popup>
>>> + </ui>
>>> +
>>
>> Please put this file into src/persp/dbgperspective/menus/ instead.
>
> I don't agree, the terminal is something that could be useful to other
> perspectives. I think that why the terminal source code is in
> src/uicommon and not in src/persp/dbgperspective.
> If you still want me to put this file in
> src/pers/dbgperspective/menus/ I will have to move the
> nmv-terminal.{cc,h} files inside the dbgperspective source directory.

Well, stricto-sensu, there should be a menus/ directory in
src/uicommon.  The menus directory under $top_srcdir is meant for stuff
that is independent from the any perspective, namely, for the topmost
menus of the workbench.  In retrospect, I should have put that into
src/workbench.  But I think shuffling these directories around is
something that ought to be done in another set of patch.

So for now I think all the menus/ should be better just put in
src/persp/dbgperspective/menus/, waiting for the cleanup pass I
mentioned above.  There are other things in src/persp/dbgperspective
that should be moved back to src/uicommon as well.  When we are at the
time of moving all that stuff there, we'll do the menu stuff.

>>> +using namespace common;
>>> +
>>> +typedef std::tr1::tuple<VteTerminal*&,
>>> +                   Gtk::Menu*&,
>>> +                   Glib::RefPtr<Gtk::ActionGroup>&> TerminalPrivDataTuple;
>>> +
>>
>> Why references to pointers (VteTerminal*& and Gtk::Menu*?)?  Why not
>> just pointers?
>
> Because I'm initializing the tuple when the vte and menu pointers are
> NULL. So the tuple will contains a copy of the NULL pointer and the
> menu won't be working.
> And the reference also assure me that if someone change for any reason
> the pointer of any of the two members, he won't have to update the
> tuple and won't forget to do it.

OK, fair enough.

-- 
		Dodji