Merge lp:~simon-kersey/bzr-explorer/568728-fix into lp:bzr-explorer

[Simon Kersey]

Fixes refresh when working tree directory renamed or deleted (Bug 568728)

Changed QExplorerMainWindow.do_refresh so that it only updates the working tree if the current location supports and has a working tree.

Refactored Location models so that they have methods to indicate that the model supports a working tree and has a working tree. Updated QExplorerMainWindow.changed_view to use this.

To test (on Windows):
- Start BzrExplorer
- Open a repository containing branches/checkouts
- Using OS copy one of the branches or checkouts within the repository
- In BzrExplorer refresh repository tab - check copied branch/checkout now shown
- Double click on the copied branch/checkout in BzrExplorer repository tab to open it
- Using OS attempt to delete copied branch/checkout: should fail whilst current tab in BzrExplorer is the copied branch/checkout (existing behaviour)
- Change current tab in BzrExplorer to be repository
- Using OS attempt to delete copied branch/checkout: should succeed
- Refresh the repository tab in BzrExplorer: deleted branch/checkout should be removed
- Change current tab in BzrExplorer to be deleted branch/checkout: "Unable to change to ..." error dialog should be displayed
- OK error dialog: tab for deleted branch/checkout should be closed

Repeat above but instead of deleting copied branch/checkout using OS rename it. The same results should be observed.

[Martin Packman (gz)]

Try and avoid adding trailing whitespace to lines, it's bad for diff and blame:
45 -<EOL>
46 + <EOL>

Is there no way of writing an automated test for this? Steps to reproduce the problem are great, but aren't going to be repeated every release to ensure the bugfix still works.

review: Abstain

[lp:~simon-kersey/bzr-explorer/568728-fix]

468. By Simon Kersey on 2010-05-06

Remove trailing whitespace

[Simon Kersey (simon-kersey)]

> Try and avoid adding trailing whitespace to lines, it's bad for diff and
> blame:
> 45 -<EOL>
> 46 + <EOL>
>

Updated to remove trailing whitespace

> Is there no way of writing an automated test for this? Steps to reproduce the
> problem are great, but aren't going to be repeated every release to ensure the
> bugfix still works.

At the moment there's not much in the way of tests for Bzr-Explorer. I don't mind trying to write some for this patch but don't have a clear idea where to start.

[lp:~simon-kersey/bzr-explorer/568728-fix]

469. By Simon Kersey on 2010-08-23

merge trunk

[Alexander Belchenko (bialix)]

Sorry for the delay. I will review it soon. (Ping to myself)

[Alexander Belchenko (bialix)]

Just one note: please don't add extra blank lines between methods, as you do in lib/explorer.py after do_refresh method. I'll remove it before merging, this note for future.

Thank you for you fix!

review: Approve

=== modified file 'NEWS'
--- NEWS	2010-08-10 12:40:22 +0000
+++ NEWS	2010-08-24 09:41:02 +0000
@@ -22,6 +22,8 @@
 * Custom switch dialog in colocated workspace works correctly with old PyQt 4.4.
   (Bug #582714, Alexander Belchenko)
 
+* Fixed refresh when directory renamed/deleted (Simon Kersey, #568728)
+
 Notes:
 
 * bzr-explorer now requires qbzr 0.19.

=== modified file 'lib/explorer.py'
--- lib/explorer.py	2010-08-06 15:08:29 +0000
+++ lib/explorer.py	2010-08-24 09:41:02 +0000
@@ -599,9 +599,11 @@
         available = self._supported_commands
         kind = None
         working_tree = None
+        wt_applicable = False
         if view:
             kind = view.kind()
-            if kind != kinds.WELCOME_PAGE:
+            wt_applicable = view.has_wt()
+            if view.supports_wt():
                 working_tree = view.working_tree()
         for action, conditions in self._conditions_by_action.items():
             try:
@@ -625,11 +627,6 @@
         # Update the working tree browser, hiding it for pages where
         # it's never applicable
         action = self.ui.action_WorkingTree_Browser
-        wt_applicable = working_tree is not None and kind in [
-            kinds.BRANCH_KIND,
-            kinds.BOUND_BRANCH_KIND,
-            kinds.CHECKOUT_KIND,
-            ]
         action.setEnabled(wt_applicable)
         wt_visible = wt_applicable and action.isChecked()
         self._wt_browser.setVisible(wt_visible)
@@ -1691,8 +1688,13 @@
 
     def do_refresh(self, reopen=False):
         self.location_viewer.refresh_current(reopen=reopen)
-        self._wt_browser.refresh()
+        #
+        # only refresh working tree if current view has one 
+        view = self.current_view()
+        if view and view.has_wt():
+            self._wt_browser.refresh()
         self.changed_view()
+        
 
     def refresh_diagnostic_mode(self):
         self.dry_run = self.ui.action_Diagnostic_Mode.isChecked()

=== modified file 'lib/location.py'
--- lib/location.py	2010-02-21 07:04:02 +0000
+++ lib/location.py	2010-08-24 09:41:02 +0000
@@ -135,6 +135,14 @@
     def group(self):
         """Return the group key or None if not part of a group."""
         return None
+    
+    def supports_wt(self):
+        """Return true if model should have a working tree"""
+        return False
+
+    def has_wt(self):
+        """Return true if model has a working tree"""
+        return False
 
 
 class ExtensibleLocationModel(_AbstractLocationModel):
@@ -275,6 +283,18 @@
         """
         return self.interesting_kind
 
+    def supports_wt(self):
+        """Return True if model should have a working tree"""
+        return (self.interesting_kind in [
+            BRANCH_KIND,
+            BOUND_BRANCH_KIND,
+            CHECKOUT_KIND,
+            ])
+
+    def has_wt(self):
+        """Return True if model has a working tree"""
+        return (self.supports_wt() and self.tree is not None)
+
     def category(self):
         """Return one of local, remote, None."""
         return {True: "local", False: "remote"}[self._local]