Re: Hello ! It's been a long time ! - patch ready (almost) for pop up issue :)

[Seemanta Dutta <seemanta gmail com>]

Hi Dodji,
First of all, please accept my apologies. I got entangled in some other personal stuff so could not send you the patch immediately after your last email. 

So without any further delay, I present you the patch, attached here in this mail. Please review it and let me know your comments. Though I have tested the fix, it would be a good idea if you could also do some testing. I don't want to cause any regression with my changes ;-).

Also, I made the change which you had suggested to know if an inferior is being debugged. Now in the code, I use debugger () ->is_attached_to_target (). 

Another good thing is that, after I upgraded to Lucid Lynx, the pop up is coming exactly in the middle and not on the top left hand corner of the screen as earlier. So that rests this issue for some time ;-)

Thanks again for your support and looking forward to hear comments from you on my patch.

regards,
Seemanta

On Thu, May 6, 2010 at 6:36 PM, Seemanta Dutta <seemanta gmail com> wrote:

Hi Dodji,
Thanks for your reply! And please, no need to apologize :) I understand everyone has priorities and other stuff to take care of, so I perfectly understand.

Please see inline for my comments.

regards,
Seemanta

On Thu, May 6, 2010 at 6:07 PM, Dodji Seketeli <dodji seketeli org> wrote:

Hello Seemanta,

Sorry for my late reply. I hate replying late to code questions like
that, but this time I am afraid I have no other way but to apologize :(

Le lundi 03 mai 2010 à 03:24:39% (+0530), Seemanta Dutta a écrit:

> Thanks for your detailed answer.

You are welcome.

[...]

> I have now implemented the yes/no dialog  behaviour and it works :D

Oh, great!

> As you suggested, I used a veto mechanism. But
> before I can send a patch to you for review, I have a few questions that
> need to be answered.

No problem.

> 1. I used LOG_DD macro for logging. But I am not seeing any log
> messages. I
> suspect I might not be turning on some macro. Can you please point me to it
> ?

In Nemiver, logs are controlled by log domains. A log domain is just a
string. You can emit a message for a log domain by using the macro:

LOG_D ("message", "log-domain-foo");

Then, when you set the environment variable NMV_LOG_DOMAINS and launch
Nemiver like this:

NMV_LOG_DOMAINS=log-domain-foo nemiver, the log "message" will be
displayed on standard output.

The macro LOG_DD is a shortcut macro that means, "Log the message for
the default (log) domain". And the default log domain there is the name
of the source code file where you used the LOG_DD macro.

Suppose you are writing code in file foo.cc, and you write
LOG_DD("boo"); The, if you want to see your logs, you have to launch
nemiver with an environment variable saying:

NMV_LOG_DOMAIN=foo.cc nemiver

The log message "boo" will appear on standard output.

This is very handy because it let's us scatter logging around in the
code, and whenever a user reports a weird bug somewhere, we can ask him
for a specific log of a specific Nemiver subsystem to understand his
problem better.

There is documentation about Nemiver logging on the wiki at:

http://live.gnome.org/Nemiver/ActivateLogging

Thanks, will do !

> 2. How do I know if an inferior is being debugged ?

The IDebugger interface has the function:
IDebugger::is_attached_to_target for that.

So, if you are in the DBGPerspective class, you can check for
debugger ()->is_attached_to_target ().

[...]

Thanks, will use this function!

> 3. The pop that comes up, comes up at my top left hand side of the
> screen.
> Not sure if this is a GTK+ specific setting on my machine or default
> behaviour of all top level pop ups. Can it be modified so that the pop up
> comes right in the center of the main workbench window ?

Hmmh. I think we could use some GTK+-fu in the function that pops up the
dialog, but that's independant from your patch. How about reviewing your
patch first, and then fix that dialog positionning issue separately?

Yes, I agree. I shall send the patch in later today for review. We can tackle this issue separately.

 

> Once I get the answers to the above questions, I can release an
> initial patch for you to review. I can upload my patch directly to
> bugzilla for this bug which is already opened, right ?

You can do that, or you can just send the patch to this list in
attachment to your email. I suspect I or Jonner (or both) will happily
review it in a timely manner :-) I prefer patches to be sent to the
mailing list, because then it's easier for me to just reply to
the patch by email and do the review like that. I like the power that
git gives us by being able to work while being disconnected. I use that
same power for email. I fetch my emails, and read them offline. I find
it inconvenient to be obliged to be connected to do patch reviews in
bugzilla.

Also, please assign the bug to yourself in bugzilla so that nobody
duplicates work trying to work on the same bug.

I shall attach the path in this mail for your review. And I think you already assigned the bug to me :)

Thank you for your time.

Thank *you* for your support :)
 

Cheers,

       Dodji

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(-)

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
@@ -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,24 @@ DBGPerspective::show_log_view_signal ()
     return m_priv->show_log_view_signal;
 }
 
+bool
+DBGPerspective::agree_to_shutdown()
+{
+    LOG_DD("In Veto function");
+    UString message;
+    message.printf (_("There is a program being currently debugged. \
+                     Do you really want to exit from the debugger?"));
+    if( debugger ()->is_attached_to_target () ) {
+        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..2d38267 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..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 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..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();
 
 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