[MERGE] Give commit a progress bar

[Robert Collins]

On Fri, 2006-04-21 at 16:49 +1000, Martin Pool wrote:
> I think this is useful but I'd really like at this point to freeze out
> any non-critical bugs so that we can get 0.8 out.  So please add this to
> a wishlist bug and keep the branch until next week when we can reopen
> for general development.

Sure.

> So +1 with comments, but not right now.
> 
> On 21 Apr 2006, Robert Collins <robertc at robertcollins.net> wrote:
> > 
> > -- 
> > GPG key available at: <http://www.robertcollins.net/keys.txt>.
> 
> > === added file 'b/bzrlib/tests/workingtree_implementations/test_commit.py'
> > +class CapturingUIFactory(ui.UIFactory):
> > +    """A UI Factory for testing - capture the updates made through it."""
> 
> Perhaps this should move (along with its selftests) to a more generic
> location, to encourage other test code to use it?
> 
> > === modified file 'a/bzrlib/commit.py'
> > --- a/bzrlib/commit.py	
> > +++ b/bzrlib/commit.py	
> > @@ -245,7 +245,12 @@
> >              self.reporter = reporter
> >  
> >          self.work_tree.lock_write()
> > +        self.pb = bzrlib.ui.ui_factory.nested_progress_bar()
> >          try:
> > +            # Cannot commit with conflicts present.
> > +            if len(self.work_tree.conflicts())>0:
> > +                raise ConflictsInTree
> > +
> >              # setup the bound branch variables as needed.
> >              self._check_bound_branch()
> 
> I'm curious - why did you move the conflict detection from below?

Bug #5798: 'conflicts in tree' check happens after commit message
editor. 

If you type in a careful email, and then get a conflict warning, it can
piss one off. I was in the area.. shrug.

>    if found_exception is not None: 
> > +            # dont do a plan raise, because the last exception may have been
> > +            # trashed, e is our sure-to-work exception even though it loses the
> > +            # full traceback. XXX: RBC 20060421 perhaps we could check the
> > +            # exc_info and if its the same one do a plain raise otherwise 
> > +            # 'raise e' as we do now.
> > +            raise e
> >  
> 
> I see this just factors out the existing code but it feels quite
> strange.  Shouldn't we just have multiple try/finally blocks, and split
> out the code if that makes it too deeply nested?

Well, its likely that it will grow further, and we have multiple places
this will be needed eventually - for instance in doCleanups [or similar]
in TestCase.tearDown(), in the multiple-unlock actions of InterFOO
classes, etc. Seems to me we should figure out the right idiom in once
place and once that is done move it to a class representing cleanups
somewhere.



> > +    def _update(self):
> > +        """Emit an update to the progress bar."""
> > +        self.pb.update("Committing", self.pb_count, self.pb_total)
> > +        self.pb_count += 1
> > +
> >      def _select_entry(self, new_ie):
> >          """Make new_ie be considered for committing."""
> >          ie = new_ie.copy()
> 
> I'd much prefer _update_pb or some such, _update is a bit generic.

sure.

>   
> > +    def clear_term(self):
> > +        """See progress.ProgressBar.clear_term()."""
> > +
> >      def finished(self):
> >          """See progress.ProgressBar.finished()."""
> 
> I (still) dislike these docstrings; they use up memory and hide the real
> docstring without adding anything useful.  (I know we talked about it
> before, no action is required for this commit.)  I would prefer if they
> could just be comments.

Do we have a bug open on the solution we came up with? Or a spec? Either
of those would be good - as a QA aspect of the code base its something I
might get around to post dapper :)

Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060421/18070183/attachment.pgp