Re: [PATCH] New Layout Manager and 3 additionals new layouts

[Dodji Seketeli <dodji seketeli org>]

> You can find attached two new patches: the first one remove the unused
> status views: command, log, and target output, and the second one is
> the layout manager updated from your first review.

Thanks.

> Sorry for taking so long to update my patch.

No problem.

> >> +    m_priv->dbg_perspective->get_terminal_view_widget ()
> >> +        .set_size_request (width, height);
> >
> >Why not using set_default_size instead of set_size_request?
> 
> Because this method only exists for a Gtk::Window

Oh, right.

> 
> > Do you want to prevent the user from shrinking the terminal view widget?
> 
> It's the opposite, I want to allow the user to shrink the terminal
> view widget.

I see.

> 
> >> +#ifdef WITH_MEMORYVIEW
> >> +    m_priv->dbg_perspective->get_memory_view_widget ()
> >> +        .set_size_request (width);
> >> +#endif // WITH_MEMORYVIEW
> >
> >Also, I don't understand why you need to set_size_request just for the
> >terminal and memory view widgets and not for the other view widgets.
> 
> I don't know why, but when you add the terminal widget to a
> Gdl::DockItem, the item takes all the vertical place (at least 700px
> on my screen) and we cannot shrink it. The consequence is you cannot
> see the sourceview widget.
> With a set_size_request the widget become shrinkable.
> I know that it is a dirty hack but it's working.
> 
> The same happens for the memory view, but not for all the other status widgets.
> I edited my comment hoping the new one is more clear.

Hmmh.  I guess the dock stuff needs some debugging.  OK, then.

> Subject: [PATCH 1/2] Remove unused status widgets
> 
> 	* src/persp/dbgperspective/menus/menus.xml: Remove commands, errors,
> 	and target output menu items.
> 	* src/persp/dbgperspective/nmv-dbg-perspective.cc
> 	(DBGPerspective::Priv::Priv): Stop initializing commands, errors,
> 	and target output datas.
> 	(DBGPerspective::on_show_commands_action): Removed
> 	(DBGPerspective::on_show_errors_action): Removed
> 	(DBGPerspective::on_show_target_output_action): Removed
> 	(DBGPerspective::on_insert_in_command_view_signal): Removed
> 	(DBGPerspective::on_show_command_view_changed_signal): Removed
> 	(DBGPerspective::on_show_target_output_view_changed_signal): Removed
> 	(DBGPerspective::on_show_log_view_changed_signal): Removed
> 	(DBGPerspective::on_debugger_console_message_signal): Removed
> 	(DBGPerspective::on_debugger_target_output_message_signal): Removed
> 	(DBGPerspective::on_debugger_log_message_signal): Removed
> 	(DBGPerspective::on_activate_output_view): Removed
> 	(DBGPerspective::on_activate_logs_view): Removed
> 	(DBGPerspective::get_command_view): Removed
> 	(DBGPerspective::get_command_view_scrolled_win): Removed
> 	(DBGPerspective::get_target_output_view): Removed
> 	(DBGPerspective::get_target_output_view_scrolled_win): Removed
> 	(DBGPerspective::get_log_view): Removed
> 	(DBGPerspective::get_log_view_scrolled_win): Removed
> 	(DBGPerspective::set_show_command_view): Removed
> 	(DBGPerspective::set_show_target_output_view): Removed
> 	(DBGPerspective::set_show_log_view): Removed
> 	(DBGPerspective::add_text_to_command_view): Removed
> 	(DBGPerspective::add_text_to_target_output_view): Removed
> 	(DBGPerspective::add_text_to_log_view): Removed
> 	(DBGPerspective::show_command_view_signal): Removed
> 	(DBGPerspective::show_target_output_view_signal): Removed
> 	(DBGPerspective::show_log_view_signal): Removed
> 	(DBGPerspective::on_debugger_command_done_signal): Stop updating
> 	the command view.
> 	(DBGPerspective::init_perspective_menu_entries): Remove code hiding
> 	the command, log, and target output view.
> 	(DBGPerspective::init_actions): Stop initializing the command, log,
> 	and target output menu entries.
> 	(DBGPerspective::init_body): Stop initializing the command, log,
> 	and target output view.
> 	(DBGPerspective::init_signals): Stop inializing the command, log,
> 	and target output signals.
> 	(DBGPerspective::init_debugger_signals): Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective.h
> 	(DBGPerspective::add_text_to_command_view): Removed
> 	(DBGPerspective::add_text_to_target_output_view): Removed
> 	(DBGPerspective::add_text_to_log_view): Removed

This patch is OK to commit to the gtk2-branch.  If it applies to
master, it would be nice to commit it there as well.


> Subject: [PATCH 2/2] New Layout Manager and 3 additionals new layouts
> 
> 	* configure.ac: Add directives to enable/disable the building of the
> 	dynamic layout.
> 	* data/schemas/gconf/nemiver-dbgperspective.schemas: Add new configuration
> 	keys for the new layouts.
> 	* data/schemas/gsettings/org.nemiver.gschema.xml: Likewise
> 	* src/confmgr/nmv-conf-keys.h: Likewise
> 	* src/confmgr/nmv-gconf-keys-defs.cc: Likewise
> 	* src/confmgr/nmv-gsettings-keys-defs.c: Likewise
> 	* src/persp/dbgperspective/Makefile.am: Add new source files
> 	* src/persp/dbgperspective/nmv-dbg-perspective-default-layout.cc: New API
> 	* src/persp/dbgperspective/nmv-dbg-perspective-default-layout.h: Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective-dynamic-layout.cc: Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective-dynamic-layout.h: Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective-two-pane-layout.cc: Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective-two-pane-layout.h: Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective-wide-layout.cc: Likewise
> 	* src/persp/dbgperspective/nmv-dbg-perspective-wide-layout.h: Likewise.
> 	* src/persp/dbgperspective/nmv-dbg-perspective.cc
> 	(DBGPerspective::on_layout_changed): New API
> 	(DBGPerspective::init_perspective_menu_entries): Removed
> 	(DBGPerspective::register_layouts): New API
> 	(DBGPerspective::get_source_view_widget): New API
> 	(DBGPerspective::set_show_context_view): Removed
> 	(DBGPerspective::set_show_terminal_view): Likewise
> 	(DBGPerspective::set_show_breakpoints_view): Likewise
> 	(DBGPerspective::set_show_registers_view): Likewise
> 	(DBGPerspective::set_show_memory_view): Likewise
> 	(DBGPerspective::activate_status_view): Removed. Implemented in each Layout.
> 	(DBGPerspective::Priv::Priv):
> 	(DBGPerspective::Priv::layout): New API
> 	(DBGPerspective::on_shutdown_signal): Call the layout's save_configuration
> 	method.
> 	(DBGPerspective::on_debugger_running_signal): Check pointer before using it
> 	(DBGPerspective::on_activate_context_view): Use the new Layout Manager API
> 	(DBGPerspective::on_activate_target_terminal_view): Likewise
> 	(DBGPerspective::on_activate_breakpoints_view): Likewise
> 	(DBGPerspective::on_activate_registers_view): Likewise
> 	(DBGPerspective::on_activate_memory_view): Likewise
> 	(DBGPerspective::init_actions): Use Renamed ViewsIndex's elements.
> 	(DBGPerspective::init_body): Move status views initialization into the new
> 	Layout API.
> 	(DBGPerspective::init_signals): Connect signal
> 	to DBGPerspective::on_layout_changed when the layout has changed.
> 	(DBGPerspective::bring_source_as_current): Check pointer before using it.
> 	(DBGPerspective::do_init): Call register_layouts to initialize the layout
> 	manager.
> 	(DBGPerspective::get_body): Use the Layout Manager to get the perspective's
> 	body.
> 	(DBGPerspective::edit_workbench_menu): Remove comment
> 	(DBGPerspective::update_file_maps): Check pointer before using it.
> 	(DBGPerspective::edit_preferences): Adapt to the new PreferencesDialog's
> 	constructor prototype.
> 	(DBGPerspective::layout_changed_signal): New API
> 	* src/persp/dbgperspective/nmv-dbg-perspective.h
> 	(DBGPerspective::get_source_view_widget): New API
> 	(DBGPerspective::get_conf_mgr): Add the method to the interface
> 	(DBGPerspective::layout_changed_signal): New API
> 	* src/persp/dbgperspective/nmv-preferences-dialog.cc
> 	(PreferencesDialog::Priv::Priv): Take the Perspective and the LayoutManager
> 	as parameters instead of a Workbench.
> 	(PreferencesDialog::Priv::init): Add the LayoutSelector widget to the
> 	preferences dialog.
> 	(PreferencesDialog::Priv::conf_manager): Use the IPerspective to get the
> 	configuration manager instead of the workbench.
> 	(PreferencesDialog::PreferencesDialog): Take the Perspective and
> 	the LayoutManager as parameters instead of a Workbench.
> 	* src/persp/dbgperspective/nmv-preferences-dialog.h
> 	(PreferencesDialog::PreferencesDialog): Take the Perspective and
> 	the LayoutManager as parameters instead of a Workbench.
> 	* src/persp/dbgperspective/ui/Makefile.am: Remove bodycontainer.ui file
> 	* src/persp/dbgperspective/ui/bodycontainer.ui: Removed
> 	* src/persp/dbgperspective/ui/preferencesdialog.ui: Add Layout tab
> 	* src/persp/nmv-i-perspective.h: Add a signal when the layout changed.
> 	* src/uicommon/Makefile.am: Add new source files and add new include
> 	directories.
> 	* src/uicommon/nmv-layout-manager.cc: New API
> 	* src/uicommon/nmv-layout-manager.h: Likewise
> 	* src/uicommon/nmv-layout-selector.cc: Likewise
> 	* src/uicommon/nmv-layout-selector.h: Likewise
> 	* src/uicommon/nmv-layout.h: Likewise
> 	* src/workbench/nmv-workbench.cc
> 	(Workbench::on_perspective_body_changed_signal): New API
> 	(Workbench::do_init): Attach the signal 'layout_changed_signal' when
> 	the layout changed to the Workbench::on_perspective_layout_changed_signal
> 	method.

[...]

> diff --git a/src/persp/dbgperspective/nmv-dbg-perspective.cc b/src/persp/dbgperspective/nmv-dbg-perspective.cc
> index f810775..ae9318b 100644
> --- a/src/persp/dbgperspective/nmv-dbg-perspective.cc
> +++ b/src/persp/dbgperspective/nmv-dbg-perspective.cc

[...]

>  void
> -DBGPerspective::activate_status_view (Gtk::Widget &a_page)
> +DBGPerspective::on_layout_changed ()
>  {
> -    int pagenum = 0;
>      LOG_FUNCTION_SCOPE_NORMAL_DD;
>  
> +    NEMIVER_TRY
> +
>      THROW_IF_FAIL (m_priv);
> -    THROW_IF_FAIL (m_priv->statuses_notebook);
>  
> -    pagenum = m_priv->statuses_notebook->page_num (a_page);
> -    if (pagenum != -1) {
> -        if (m_priv->statuses_notebook->get_current_page () != pagenum)
> -            m_priv->statuses_notebook->set_current_page (pagenum);
> -        a_page.grab_focus ();
> -    } else {
> -        LOG_DD ("Invalid Pagenum");
> -    }
> +#ifdef WITH_MEMORYVIEW
> +    m_priv->layout ().add_view (get_memory_view ().widget (),
> +                                MEMORY_VIEW_TITLE,
> +                                MEMORY_VIEW_INDEX);
> +#endif // WITH_MEMORYVIEW
> +    m_priv->layout ().add_view (get_registers_scrolled_win (),
> +                                REGISTERS_VIEW_TITLE,
> +                                REGISTERS_VIEW_INDEX);
> +    m_priv->layout ().add_view (get_breakpoints_scrolled_win (),
> +                                BREAKPOINTS_VIEW_TITLE,
> +                                BREAKPOINTS_VIEW_INDEX);
> +    m_priv->layout ().add_view (get_context_paned (),
> +                                CONTEXT_VIEW_TITLE,
> +                                CONTEXT_VIEW_INDEX);
> +    m_priv->layout ().add_view (get_terminal_box (),
> +                                TARGET_TERMINAL_VIEW_TITLE,
> +                                TARGET_TERMINAL_VIEW_INDEX);
> +
> +    m_priv->layout ().do_init ();

I think it would be appropriate to put this code that adds the views
to the layout into a function DBGPerspective::add_views_to_layout,
then call that function here.

> +
> +    NEMIVER_CATCH
>  }
>

> @@ -3863,13 +3779,27 @@ DBGPerspective::init_body ()
>      get_breakpoints_scrolled_win ().add (get_breakpoints_view ().widget());
>      get_registers_scrolled_win ().add (get_registers_view ().widget());
>  
> -    //unparent the body_main_paned, so that we can pack it
> -    //in the workbench later
> -    m_priv->top_box->remove (*m_priv->body_main_paned);
> -    m_priv->body_main_paned->show_all ();
> +    m_priv->sourceviews_notebook.reset (new Gtk::Notebook);
> +    m_priv->sourceviews_notebook->remove_page ();
> +    m_priv->sourceviews_notebook->set_show_tabs ();
> +    m_priv->sourceviews_notebook->set_scrollable ();
> +#if GTK_CHECK_VERSION (2, 10, 0)
> +    m_priv->sourceviews_notebook->signal_page_reordered ().connect
> +        (sigc::mem_fun (this, &DBGPerspective::on_notebook_tabs_reordered));
> +#endif
> +
> +    UString layout = DBG_PERSPECTIVE_DEFAULT_LAYOUT;
> +    NEMIVER_TRY
> +    conf_mgr.get_key_value (CONF_KEY_DBG_PERSPECTIVE_LAYOUT, layout);
> +
> +    // If the layout is not registered, we use the default layout
> +    if (!m_priv->layout_mgr.is_layout_registered (layout)) {
> +        layout = DBG_PERSPECTIVE_DEFAULT_LAYOUT;
> +    }
> +    NEMIVER_CATCH_NOX
>  
> -    //must be last
> -    init_perspective_menu_entries ();
> +    m_priv->layout_mgr.load_layout (layout, *this);
> +    on_layout_changed ();

Instead of calling on_layout_changed here, it would make more sense
(from a maintainability POV) to call the function add_views_to_layout
I talked about earlier.  This would make what we are doing more
self-explanatory.

This patch looks OK for me to commit to gtk2-branch, once you have
addressed the minor nits I referred to above.  If you haven't run it
through Valgrind yet, please do so.

By the way, how about porting it to trunk?

-- 
		Dodji