Re: Hello ! It's been a long time ! - patch ready (almost) for pop up issue :)
- From: Seemanta Dutta <seemanta gmail com>
- To: Dodji Seketeli <dodji seketeli org>
- Cc: nemiver list <nemiver-list gnome org>
- Subject: Re: Hello ! It's been a long time ! - patch ready (almost) for pop up issue :)
- Date: Sun, 23 May 2010 00:31:19 +0530
Hi Dodji,
Thanks for reviewing my code. I apologize for missing to follow the proper coding guidelines. I have corrected all my mistakes and sending the corrected patch with this email. I have amended my earlier patch with these new fixes. So this patch is the entire fix, and is not just the corrections you mentioned.
And as always, thanks for your support.
regards,
Seemanta
On Fri, May 21, 2010 at 1:49 PM, Dodji Seketeli
<dodji seketeli org> wrote:
Hello Seemanta,
Thanks for the patch. It looks good to me in general and from my
testing, it works like a charm! I have however some minor nits to
point out, so here go my comments.
[...]
> From 178c5e7277725137d3a92a7f2216229cd79c6dcf Mon Sep 17 00:00:00 2001
> From: Seemanta Dutta <seemanta gmail com>
> Date: Sun, 16 May 2010 13:55:22 +0530
> Subject: [PATCH] Adding a popup when quitting nemiver.
>
> ---
> src/persp/dbgperspective/nmv-dbg-perspective.cc | 20 ++++++++++++++
> src/persp/dbgperspective/nmv-dbg-perspective.h | 2 +
> src/persp/nmv-i-perspective.h | 10 +++++++
> src/workbench/nmv-workbench.cc | 32 ++++++++++++++++++++---
> 4 files changed, 60 insertions(+), 4 deletions(-)
It would have been nice to write a log describing the patch. There is
a git-commit-messages.README file at the top level of hte source tree
that describes what the log should loo like. You can also do a git log
and see what the other commit logs look like.
> diff --git a/src/persp/dbgperspective/nmv-dbg-perspective.cc b/src/persp/dbgperspective/nmv-dbg-perspective.cc
> index 5841f19..f09a0c8 100644
> --- a/src/persp/dbgperspective/nmv-dbg-perspective.cc
> +++ b/src/persp/dbgperspective/nmv-dbg-perspective.cc
[...]
> +bool
> +DBGPerspective::agree_to_shutdown()
> +{
> + LOG_DD("In Veto function");
There should be a space before the parenthesis.
> + UString message;
> + message.printf (_("There is a program being currently debugged. \
> + Do you really want to exit from the debugger?"));
Declaring and setting this variable should probably better be done in
the if-block below.
> + if( debugger ()->is_attached_to_target () ) {
> + if (nemiver::ui_utils::ask_yes_no_question (message) == Gtk::RESPONSE_YES) {
This line is longer than 80 characters. Please keep lines shorter than
80 characters.
Also, there should be a space between the parenthesis and the if, and
no space between the parenthesis and debugger. There shouldn't be any
space between target () and the closing parenthesis of the if either.
These things are described in the coding-style.txt file at the top
level of the source directory.
[...]
> diff --git a/src/persp/nmv-i-perspective.h b/src/persp/nmv-i-perspective.h
> index fad359d..1cc32dd 100644
> --- a/src/persp/nmv-i-perspective.h
> +++ b/src/persp/nmv-i-perspective.h
> @@ -121,6 +121,16 @@ public:
> virtual Gtk::Widget* load_menu (const UString &a_filename,
> const UString &a_widget_name) = 0;
>
> + /// \brief method which will be called for each perspective before
> + /// workbench initiates a shutdown(). This is a chance given to
The \brief comment should stand on one line.
The next line would then be the begining of the longer form of the
brief, one-liner comment.
So it should probably be something like.
/// \brief Should returns true to allow shutdown.
// This method is called for each perspective before the workbench
initiates a shutdown() ....
[...]
> + virtual bool agree_to_shutdown() = 0;
> diff --git a/src/workbench/nmv-workbench.cc b/src/workbench/nmv-workbench.cc
> index 31f68bd..8e09f3e 100644
> --- a/src/workbench/nmv-workbench.cc
> +++ b/src/workbench/nmv-workbench.cc
> @@ -132,6 +132,7 @@ private:
> WorkbenchStaticInit::do_init ();
> }
> bool on_delete_event (GdkEventAny* event);
> + bool query_for_shutdown();
There should be a space between before the opening parenthesis.
> +bool
> +Workbench::query_for_shutdown()
Space before the open parenthesis.
> +{
> + bool retval = true;
> + list<IPerspectiveSafePtr>::const_iterator iter;
> + for (iter = m_priv->perspectives.begin ();
> + iter != m_priv->perspectives.end ();
> + ++iter) {
> + if((*iter)->agree_to_shutdown() == false) {
Space before the open parenthesis.
> + retval = false;
> + break;
> + }
> + }
> + return retval;
> +}
> +
>
> //*********************
> //signal slots methods
> @@ -197,6 +214,7 @@ bool
> Workbench::on_delete_event (GdkEventAny* a_event)
> {
> LOG_FUNCTION_SCOPE_NORMAL_DD;
> + bool retval = true;
>
> NEMIVER_TRY
> // use event so that compilation doesn't fail with -Werror :(
> @@ -204,11 +222,15 @@ Workbench::on_delete_event (GdkEventAny* a_event)
>
> // clicking the window manager's X and shutting down the with Quit menu item
> // should do the same thing
> - on_quit_menu_item_action ();
> + if(query_for_shutdown() == true) {
Space after the if.
So, would you mind sending an updated patch that addresses my comments
above? Or would you rather prefer that I do the changes myself?
In any case, thanks for your time and the nice patch.
Cheers.
Dodji
From 66c1013e4de1fda1c66d768a760c2099af0031b7 Mon Sep 17 00:00:00 2001
From: Seemanta Dutta <seemanta gmail com>
Date: Sun, 16 May 2010 13:55:22 +0530
Subject: [PATCH] Adding a popup when quitting nemiver. (Closes: #553217)
* src/persp/dbgperspective/nmv-dbg-perspective.cc
(DBGPerspective::agree_to_shutdown ()): New function
* src/persp/dbgperspective/nmv-dbg-perspective.h: New function
declaration - agree_to_shutdown ()
* src/persp/nmv-i-perspective.h: New function declaration -
agree_to_shutdown ()
* src/workbench/nmv-workbench.cc
(Workbench::query_for_shutdown ()): New function called from
Workbench::on_delete_event () (when user click on the window manager
'Cross') and Workbench::on_quit_menu_item_action () (when user
selects File->Quit).
---
src/persp/dbgperspective/nmv-dbg-perspective.cc | 21 +++++++++++++++
src/persp/dbgperspective/nmv-dbg-perspective.h | 2 +
src/persp/nmv-i-perspective.h | 11 ++++++++
src/workbench/nmv-workbench.cc | 32 ++++++++++++++++++++---
4 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/src/persp/dbgperspective/nmv-dbg-perspective.cc b/src/persp/dbgperspective/nmv-dbg-perspective.cc
index 5841f19..4de7d8c 100644
--- a/src/persp/dbgperspective/nmv-dbg-perspective.cc
+++ b/src/persp/dbgperspective/nmv-dbg-perspective.cc
@@ -705,6 +705,8 @@ public:
bool do_unmonitor_file (const UString &a_path);
void activate_status_view(Gtk::Widget& page);
+
+ bool agree_to_shutdown ();
sigc::signal<void, bool>& show_command_view_signal ();
sigc::signal<void, bool>& show_target_output_view_signal ();
@@ -7239,6 +7241,25 @@ DBGPerspective::show_log_view_signal ()
return m_priv->show_log_view_signal;
}
+bool
+DBGPerspective::agree_to_shutdown ()
+{
+ LOG_DD ("In Veto function");
+ if (debugger ()->is_attached_to_target ()) {
+ UString message;
+ message.printf (_("There is a program being currently debugged. " \
+ "Do you really want to exit from the debugger?"));
+ if (nemiver::ui_utils::ask_yes_no_question (message) == \
+ Gtk::RESPONSE_YES) {
+ return true;
+ } else {
+ return false;
+ }
+ } else {
+ return true;
+ }
+}
+
class DBGPerspectiveModule : DynamicModule {
public:
diff --git a/src/persp/dbgperspective/nmv-dbg-perspective.h b/src/persp/dbgperspective/nmv-dbg-perspective.h
index 4d1db34..4d51f1a 100644
--- a/src/persp/dbgperspective/nmv-dbg-perspective.h
+++ b/src/persp/dbgperspective/nmv-dbg-perspective.h
@@ -167,6 +167,8 @@ public:
virtual sigc::signal<void, bool>& activated_signal () = 0;
virtual sigc::signal<void, bool>& debugger_ready_signal () = 0;
+
+ virtual bool agree_to_shutdown () = 0;
};//end class IDBGPerspective
NEMIVER_END_NAMESPACE (nemiver)
diff --git a/src/persp/nmv-i-perspective.h b/src/persp/nmv-i-perspective.h
index fad359d..b8ca7f0 100644
--- a/src/persp/nmv-i-perspective.h
+++ b/src/persp/nmv-i-perspective.h
@@ -121,6 +121,17 @@ public:
virtual Gtk::Widget* load_menu (const UString &a_filename,
const UString &a_widget_name) = 0;
+ /// \brief Should return true to allow shutdown.
+ /// This Method will be called for each perspective before
+ /// workbench initiates a shutdown (). This is a chance given to
+ /// the perspective to veto the shutdown (). Each perspective has
+ /// to implement this function wherein it can decide for itself
+ /// whether it wants to veto the shutdown or not. Returning 'true'
+ /// from here means that the perspective is ok with the shutdown
+ /// returning 'false' vetoes the shutdown and nemiver does not go
+ /// down.
+ virtual bool agree_to_shutdown () = 0;
+
/// \name signals
/// @{
diff --git a/src/workbench/nmv-workbench.cc b/src/workbench/nmv-workbench.cc
index 31f68bd..a0a7c5b 100644
--- a/src/workbench/nmv-workbench.cc
+++ b/src/workbench/nmv-workbench.cc
@@ -132,6 +132,7 @@ private:
WorkbenchStaticInit::do_init ();
}
bool on_delete_event (GdkEventAny* event);
+ bool query_for_shutdown ();
public:
Workbench (DynamicModule *a_dynmod);
@@ -189,6 +190,22 @@ struct Workbench::Priv {
//****************
//private methods
//****************
+bool
+Workbench::query_for_shutdown ()
+{
+ bool retval = true;
+ list<IPerspectiveSafePtr>::const_iterator iter;
+ for (iter = m_priv->perspectives.begin ();
+ iter != m_priv->perspectives.end ();
+ ++iter) {
+ if ((*iter)->agree_to_shutdown () == false) {
+ retval = false;
+ break;
+ }
+ }
+ return retval;
+}
+
//*********************
//signal slots methods
@@ -197,6 +214,7 @@ bool
Workbench::on_delete_event (GdkEventAny* a_event)
{
LOG_FUNCTION_SCOPE_NORMAL_DD;
+ bool retval = true;
NEMIVER_TRY
// use event so that compilation doesn't fail with -Werror :(
@@ -204,11 +222,15 @@ Workbench::on_delete_event (GdkEventAny* a_event)
// clicking the window manager's X and shutting down the with Quit menu item
// should do the same thing
- on_quit_menu_item_action ();
+ if (query_for_shutdown () == true) {
+ shut_down ();
+ retval = false;
+ }
+
NEMIVER_CATCH
- //keep propagating
- return false;
+ //Will propagate if retval = false, else not
+ return retval;
}
void
@@ -218,7 +240,9 @@ Workbench::on_quit_menu_item_action ()
NEMIVER_TRY
- shut_down ();
+ if (query_for_shutdown () == true) {
+ shut_down ();
+ }
NEMIVER_CATCH
}
--
1.7.0.4
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
