Merge lp:~abudden/bzr-explorer/bound-branch-submit-pane into lp:bzr-explorer

[Dr Al]

Possibly slightly controversial, but this works round Bug 777108 by using the parent location in the submit pane for bound branches. If and when lp:~abudden/bzr/switch_parent_513709 (related to Bug 513709) lands and the parent branch is set correctly, this should make the feature branch model with bound branches much more usable.

[lp:~abudden/bzr-explorer/bound-branch-submit-pane]

503. By Dr Al on 2011-05-04

Added better support for feature branch differences: when using bound branches, show parent location rather than submit location.

504. By Dr Al on 2011-05-04

Updated NEWS.

[Alexander Belchenko (bialix)]

Should bzr-explorer check for some specific bzr version then to ensure that your fix was landed?

[Dr Al (abudden)]

On Wednesday, 4 May 2011, Alexander Belchenko <email address hidden> wrote:
> Should bzr-explorer check for some specific bzr version then to ensure that your fix was landed?

I think that would probably be sensible (it can easily be integrated).
I'll add that when (and if) the fix lands. I just thought it would be
good to get the review started early.

Al

> --
> https://code.launchpad.net/~abudden/bzr-explorer/bound-branch-submit-pane/+merge/59959
> You are the owner of lp:~abudden/bzr-explorer/bound-branch-submit-pane.
>

--
http://sites.google.com/site/abudden

[Dr Al (abudden)]

The bug fix for Bug 513709 has been approved for inclusion in 2.4b3.

A bit more investigation on the Bazaar version: I don't think it is too critical after all. The fix for Bug 513709 sets the correct parent location for both the new master (parent) branch and for the working (bound) branch. Bazaar Explorer looks at the working (bound) branch when identifying the master (parent) branch. Before the patch was landed, the parent location for the master branch was set incorrectly (which is mostly irrelevant to Bazaar Explorer) and the parent location for the working (bound) branch wasn't set at all. Therefore, in branches created before 2.4b3, the submit pane won't be shown and no difference will be seen (there's nothing we can do about this sadly). With this patch, in branches created after 2.4b3, the submit pane will be shown and the correct parent will be visible. Therefore, I don't think we need to check the Bazaar version.

I hope all that makes sense!

[Alexander Belchenko (bialix)]

Y trust your decision here. Make sure you have tested the bzr-explorer behavior with and without your patch and let's land this.

[Alexander Belchenko (bialix)]

Alexander Belchenko пишет:
> Y trust your decision here. Make sure you have tested the bzr-explorer behavior with and without your patch and let's land this.
s/Y/I/

--
All the dude wanted was his rug back

[Alexander Belchenko (bialix)]

Al, I don't like that submit pane and don't use it. One day I want to add config option to hide it forever.

But if it important for you -- then please, test t as you think appropriate, with bound branches and plain branches, and if you're happy with result, then please land it.

[Alexander Belchenko (bialix)]

review: Abstain

[Alexander Belchenko (bialix)]

I don't understand some parts of your patch. Don't land it, please, I'd like to ask something.

review: Needs Information

[Dr Al (abudden)]

No problem, I haven't had a chance to do extensive testing, so I wasn't going to just yet anyway!

[Dr Al (abudden)]

Okay, I've done quite a bit of testing now and haven't found any problems. What did you want to ask?

[Alexander Belchenko (bialix)]

OK, I think we can do a better job now and provide better label for the case when there is no submit_branch set. Right now for non-bound branches we have:

submit_url = branch.get_submit_branch() or branch.get_parent()

but the target is always target = 'submit'

Maybe we can provide correct label here as of

submit_url = branch.get_submit_branch()
if submit_url:
  target = 'submit'
  name = gettext('Submit delta')
else:
  submit_url = branch.get_parent()
  target = 'parent'
  name = gettext('Parent delta')

[Dr Al (abudden)]

On 12 May 2011 16:06, Alexander Belchenko <email address hidden> wrote:
> OK, I think we can do a better job now and provide better label for the case when there is no submit_branch set. Right now for non-bound branches we have:
>
> submit_url = branch.get_submit_branch() or branch.get_parent()
>
> but the target is always target = 'submit'
>
> Maybe we can provide correct label here as of
>
> submit_url = branch.get_submit_branch()
> if submit_url:
>  target = 'submit'
>  name = gettext('Submit delta')
> else:
>  submit_url = branch.get_parent()
>  target = 'parent'
>  name = gettext('Parent delta')

Sounds reasonable: I'll make that change.

Al

[lp:~abudden/bzr-explorer/bound-branch-submit-pane]

505. By Dr Al on 2011-05-13

Merge trunk

506. By Dr Al on 2011-05-13

Report correct target name if using parent branch.

[Dr Al (abudden)]

Done.

[Alexander Belchenko (bialix)]

Thank you, I'll merge.

review: Approve

=== modified file 'NEWS'
--- NEWS	2011-05-09 20:53:45 +0000
+++ NEWS	2011-05-13 07:21:10 +0000
@@ -38,6 +38,10 @@
 
 * Bulgarian translation added.
 
+Bug Fixes:
+
+* Use the parent branch in the submit pane when working with bound branches.
+  (A. S. Budden, Bug #777108)
 
 1.1.2 26-Nov-2010
 -----------------

=== modified file 'lib/status_data.py'
--- lib/status_data.py	2011-05-08 18:49:29 +0000
+++ lib/status_data.py	2011-05-13 07:21:10 +0000
@@ -25,6 +25,8 @@
 
 from bzrlib import errors, revisionspec, status
 
+from bzrlib import branch as mod_branch
+
 
 def get_status_info(wt, specific_files=None, versioned=False):
     """Get the status information for a working tree.
@@ -101,3 +103,13 @@
     rev_spec = revisionspec.RevisionSpec.from_string('submit:')
     old_tree = rev_spec.as_tree(branch)
     return changes_between_trees(new_tree, old_tree)[0]
+
+def get_parent_submission_info(branch):
+    """Get the status information for a branch versus its submit branch.
+
+    :return: delta
+    """
+    new_tree = branch.basis_tree()
+    old_branch = mod_branch.Branch.open_containing(branch.get_parent())[0]
+    old_tree = old_branch.basis_tree()
+    return changes_between_trees(new_tree, old_tree)[0]

=== modified file 'lib/status_report.py'
--- lib/status_report.py	2011-05-08 18:49:29 +0000
+++ lib/status_report.py	2011-05-13 07:21:10 +0000
@@ -271,10 +271,6 @@
 class SubmitReport(_BaseStatusReport):
 
     # Formatters by section
-    def _added_formatter(items):
-        # Diff is useful here because it shows what's committed which, unlike
-        # the plain status case, may vary from the current file contents
-        return _edit_with_actions_decorator(items[0], ['diff(-rsubmit:..-1)'])
     def _deleted_formatter(items):
         return items[0]
     def _renamed_formatter(items):
@@ -283,25 +279,44 @@
     def _kind_changed_formatter(items):
         text = "%s (%s => %s)" % (items[0], items[2], items[3])
         return _edit_decorator(items[0], text)
-    def _modified_formatter(items):
-        return _edit_with_actions_decorator(items[0], ['diff(-rsubmit:..-1)'])
+
+    # Create formatters on the fly where diff is used as
+    # the ancestor branch varies
+    def _make_diff_based_formatter(self):
+        if self.target == 'parent':
+            parent_branch = self.branch.get_parent()
+            diff_path = 'diff(-rancestor:%s..-1)' % parent_branch
+        else:
+            diff_path = 'diff(-rsubmit:..-1)'
+        def _diff_based_formatter(items):
+            return _edit_with_actions_decorator(items[0], [diff_path])
+        return _diff_based_formatter
+
     formatter_map = {
-        'Added':        _added_formatter,
+        'Added':        None, # Added in _collect_data
         'Deleted':      _deleted_formatter,
         'Renamed':      _renamed_formatter,
         'Kind changed': _kind_changed_formatter,
-        'Modified':     _modified_formatter,
+        'Modified':     None, # Added in _collect_data
     }
 
-    def __init__(self, branch):
+    def __init__(self, branch, target='submit'):
         self.branch = branch
+        self.target = target
         self._collect_data()
 
     def _collect_data(self):
         self._data = None
         self._exception = None
         try:
-            delta = status_data.get_submission_info(self.branch)
+            if self.target == 'submit':
+                delta = status_data.get_submission_info(self.branch)
+            elif self.target == 'parent':
+                delta = status_data.get_parent_submission_info(self.branch)
+            else:
+                raise errors.BzrError("Unrecognised target type")
+            self.formatter_map['Added'] = self._make_diff_based_formatter()
+            self.formatter_map['Modified'] = self._make_diff_based_formatter()
             if delta.has_changed():
                 self._data = delta
         except errors.BzrError, ex:
@@ -312,10 +327,10 @@
             return html_status_message(ERROR_STATUS, unicode(self._exception))
         elif self._data:
             indicator = WARNING_STATUS
-            msg = gettext("Branch has changes not present in its submit branch.")
+            msg = gettext("Branch has changes not present in its %s branch." % self.target)
         else:
             indicator = GOOD_STATUS
-            msg = gettext("Branch is fully merged into its submit branch.")
+            msg = gettext("Branch is fully merged into its %s branch." % self.target)
         return html_status_message(indicator, msg)
 
     def body_lines(self):
@@ -337,6 +352,6 @@
             delta = self._data
             if delta.has_changed():
                 open_parent_msg = gettext(
-                    "Open parent branch and merge these changes")
+                    "Open %s branch and merge these changes" % self.target)
                 lines.append(_text_decorator(open_parent_msg))
         return lines

=== modified file 'lib/view_submit.py'
--- lib/view_submit.py	2011-05-08 18:49:29 +0000
+++ lib/view_submit.py	2011-05-13 07:21:10 +0000
@@ -28,7 +28,10 @@
     shown using -rsubmit:..-1, i.e. what's been committed so far
     versus the submit branch.
     """
+    def __init__(self, model, action_callback, target):
+        super(SubmitView, self).__init__(model, action_callback)
+        self.target = target
 
     def report_text(self):
-        report = status_report.SubmitReport(self._model.branch)
+        report = status_report.SubmitReport(self._model.branch, self.target)
         return report.full_report()

=== modified file 'lib/view_workingtree.py'
--- lib/view_workingtree.py	2009-12-09 23:43:32 +0000
+++ lib/view_workingtree.py	2011-05-13 07:21:10 +0000
@@ -39,20 +39,32 @@
         # value ones which everyone ought to want. :-)
         self.detail_views = []
         branch = self.model.branch
-        # Display the delta versus the submit branch iff one is
+        # Display the delta versus the submit or parent branch iff one is
         # defined and it's local. Unfortunately though, merging a feature
         # branch into trunk sets the merged-from branch as the submit
         # branch which means we see a "silly" submit delta panel for trunk.
-        # To work around that, we don't show the panel for bound branches
-        # as trunk ought to be bound. We could look for the name
-        # "trunk" instead but that sounds even more of a hack. :-(
+        # To work around that, we show the parent location instead of the
+        # submit location for a bound branch.
         if branch.get_bound_location() is None:
-            submit_url = branch.get_submit_branch() or branch.get_parent()
-            if submit_url and submit_url.startswith("file:"):
-                self.detail_views.append(
-                    (view_submit.SubmitView(model, action_callback,),
-                        gettext("Submit delta")),
-                    )
+            submit_url = branch.get_submit_branch()
+            if submit_url:
+                target = 'submit'
+                name = gettext('Submit delta')
+            else:
+                submit_url = branch.get_parent()
+                target = 'parent'
+                name = gettext('Parent delta')
+        else:
+            target = 'parent'
+            name = gettext('Parent delta')
+            submit_url = branch.get_parent()
+
+        if submit_url and submit_url.startswith("file:"):
+            self.detail_views.append(
+                (view_submit.SubmitView(model, action_callback, target),
+                    name),
+                )
+
         # TODO: add other panels to show what's on the shelf, etc.
 
     def ui(self):