[Crash-catcher] [PATCH] GUI: stay at selected line when crashlist changes

[Jiri Moskovcak]

improves the gui behaviour so the cursor statys at the selected line 
even when the dumplist is reloaded and continues on next line when dump 
is deleted (it used to jump to the first line which is not very intuitive).

Please review.

J.

[Denys Vlasenko]

On Fri, 2010-03-19 at 14:11 +0100, Jiri Moskovcak wrote:
>  improves the gui behaviour so the cursor statys at the selected line
>
>  even when the dumplist is reloaded and continues on next line when dump 
>  is deleted (it used to jump to the first line which is not very intuitive). 

diff --git a/src/Gui/CCMainWindow.py b/src/Gui/CCMainWindow.py
index 8e8d693..b4fe3b6 100644
--- a/src/Gui/CCMainWindow.py
+++ b/src/Gui/CCMainWindow.py
@@ -229,15 +229,29 @@ class MainWindow():
         else:
             lReported.set_markup(_("<b>Not reported!</b>"))
 
+    def mark_last_selected_row(self, model, path, iter, last_selected_dump):

I suggest renaming "model" to list_store, and adding comment:
         # Get dump object from list (in our list it's in last col)

+        dump = model.get_value(iter, model.get_n_columns()-1)
+        if dump.getUUID() == last_selected_dump.getUUID():

Since you only ever use last_selected_dump.getUUID(),
you can pass last_selected_dump.getUUID() as last_selected_uuid
instead of passing whole last_selected_dump

+            self.dlist.set_cursor(model.get_path(iter)[0])
+            return True

add comment:  return True  # done, stop iteration

+        return False
+
     def on_bDelete_clicked(self, button, treeview):
+        last_dump = None

Move this line down...

         dumpsListStore, path = self.dlist.get_selection().get_selected_rows()
         if not path:
             return
-        # this should work until we keep the row object in the last position
+        # this should work until we keep the dump object in the last position
         dump = dumpsListStore.get_value(dumpsListStore.get_iter(path[0]),
dumpsListStore.get_n_columns()-1)
+        next_iter = dumpsListStore.iter_next(dumpsListStore.get_iter(path[0]))

... to here. It's easier to understand it this way that last_dump is not used
(because it doesn't even exists) in all intervening code block

+        if next_iter:
+            last_dump = dumpsListStore.get_value(next_iter,
dumpsListStore.get_n_columns()-1)
         try:
             self.ccdaemon.DeleteDebugDump("%s:%s" % (dump.getUID(),
dump.getUUID()))
             self.hydrate()
+            if last_dump:
+                # we deleted the selected line, so we want to select the next one
+                dumpsListStore.foreach(self.mark_last_selected_row, last_dump)
             treeview.emit("cursor-changed")
         except Exception, ex:
             print ex
@@ -248,11 +262,15 @@ class MainWindow():
     def on_data_changed_cb(self, *_args):
         # FIXME mark the new entry somehow....
         # remember the selected row
+        dump = None

Use the same name - last_dump - as in previous place.

         dumpsListStore, path = self.dlist.get_selection().get_selected_rows()
+        if path and dumpsListStore:
+            dump = dumpsListStore.get_value(dumpsListStore.get_iter(path[0]),
dumpsListStore.get_n_columns()-1)
         self.hydrate()
-        if not path:
-            return
-        self.dlist.set_cursor(path[0])
+        if dump:
+            # re-select the line that was selected before a new crash happened
+            dumpsListStore.foreach(self.mark_last_selected_row, dump)
+
 
     def on_bReport_clicked(self, button):
         dumpsListStore, path = self.dlist.get_selection().get_selected_rows()


-- 
vda

[Jiri Moskovcak]

Patch v2 - includes all suggestions.

J.

On 03/19/2010 05:24 PM, Denys Vlasenko wrote:
>  On Fri, 2010-03-19 at 14:11 +0100, Jiri Moskovcak wrote:
> > improves the gui behaviour so the cursor statys at the selected line
> > even when the dumplist is reloaded and continues on next line when dump
> > is deleted (it used to jump to the first line which is not very intuitive).
>
>
>  diff --git a/src/Gui/CCMainWindow.py b/src/Gui/CCMainWindow.py
>  index 8e8d693..b4fe3b6 100644
>  --- a/src/Gui/CCMainWindow.py
>  +++ b/src/Gui/CCMainWindow.py
>  @@ -229,15 +229,29 @@ class MainWindow():
>            else:
>                lReported.set_markup(_("<b>Not reported!</b>"))
>
>  +    def mark_last_selected_row(self, model, path, iter, last_selected_dump):
>
>  I suggest renaming "model" to list_store, and adding comment:
>            # Get dump object from list (in our list it's in last col)
>
>  +        dump = model.get_value(iter, model.get_n_columns()-1)
>  +        if dump.getUUID() == last_selected_dump.getUUID():
>
>  Since you only ever use last_selected_dump.getUUID(),
>  you can pass last_selected_dump.getUUID() as last_selected_uuid
>  instead of passing whole last_selected_dump
>
>  +            self.dlist.set_cursor(model.get_path(iter)[0])
>  +            return True
>
>  add comment:  return True  # done, stop iteration
>
>  +        return False
>  +
>        def on_bDelete_clicked(self, button, treeview):
>  +        last_dump = None
>
>  Move this line down...
>
>            dumpsListStore, path = self.dlist.get_selection().get_selected_rows()
>            if not path:
>                return
>  -        # this should work until we keep the row object in the last position
>  +        # this should work until we keep the dump object in the last position
>            dump = dumpsListStore.get_value(dumpsListStore.get_iter(path[0]),
> dumpsListStore.get_n_columns()-1)
>  +        next_iter = dumpsListStore.iter_next(dumpsListStore.get_iter(path[0]))
>
>  ... to here. It's easier to understand it this way that last_dump is not used
>  (because it doesn't even exists) in all intervening code block
>
>  +        if next_iter:
>  +            last_dump = dumpsListStore.get_value(next_iter,
> dumpsListStore.get_n_columns()-1)
>            try:
>                self.ccdaemon.DeleteDebugDump("%s:%s" % (dump.getUID(),
> dump.getUUID()))
>                self.hydrate()
>  +            if last_dump:
>  +                # we deleted the selected line, so we want to select the next one
>  +                dumpsListStore.foreach(self.mark_last_selected_row, last_dump)
>                treeview.emit("cursor-changed")
>            except Exception, ex:
>                print ex
>  @@ -248,11 +262,15 @@ class MainWindow():
>        def on_data_changed_cb(self, *_args):
>            # FIXME mark the new entry somehow....
>            # remember the selected row
>  +        dump = None
>
>  Use the same name - last_dump - as in previous place.
>
>            dumpsListStore, path = self.dlist.get_selection().get_selected_rows()
>  +        if path and dumpsListStore:
>  +            dump = dumpsListStore.get_value(dumpsListStore.get_iter(path[0]),
> dumpsListStore.get_n_columns()-1)
>            self.hydrate()
>  -        if not path:
>  -            return
>  -        self.dlist.set_cursor(path[0])
>  +        if dump:
>  +            # re-select the line that was selected before a new crash happened
>  +            dumpsListStore.foreach(self.mark_last_selected_row, dump)
>  +
>
>        def on_bReport_clicked(self, button):
>            dumpsListStore, path = self.dlist.get_selection().get_selected_rows()
>
>

[Denys Vlasenko]

On Fri, 2010-03-19 at 18:02 +0100, Jiri Moskovcak wrote:
>  Patch v2 - includes all suggestions. 
Looks good.
-- 
vda

[Jiri Moskovcak]

On 03/19/2010 06:33 PM, Denys Vlasenko wrote:
>  On Fri, 2010-03-19 at 18:02 +0100, Jiri Moskovcak wrote:
> > Patch v2 - includes all suggestions.
>
>  Looks good. 
Pushed, thx for the review.