[MERGE][#172612] bzr commit: don't print the revision number twice

Matt Nordhoff mnordhoff at mattnordhoff.com
Sat Dec 1 03:10:15 GMT 2007


Alexander Belchenko wrote:
> Matt Nordhoff ?8H5B:
>> Alexander Belchenko wrote:
>>> Matt Nordhoff ?8H5B:
>>>> Alexander Belchenko wrote:
>>>>> Matt Nordhoff ?8H5B:
>>>>>> Alexander Belchenko wrote:
>>>> In the case where no location is passed, there would be an extra space
>>>> at the end of the message. The location is never not passed, and nobody
>>>> would see the space (and my shell seems to remove it entirely), but that
>>>> would still bother me.
>>> IIUC, location *should* be passed all the time.
>>> Otherwise why for you deprecate old behavior?
>> location is and always has been passed all the time, at least from
>> bzrlib. But deprecating means it should still work. I guess you have a
>> point that an extra space is even less important when it's only from
>> deprecated behaviour that never happens anyway.
> 
> Bug #172612 is about noise in output. IMO we should go further and fix
> another noise factor, i.e. when no location specified. In this case
> your patch will produce line with single word:
> 
> Committing
> 
> and that's all. IMO it's a bit strange and incomplete.
> There was revno in this line
> if no location specified, but now it's just floating alone.
> We could remove it in this case completely. I.e.:
> 
>      def started(self, revno, rev_id, location=None):
>          if location is not None:
> -            location = ' to "' + unescape_for_display(location, 'utf-8') + '"'
> +            self._note('Committing to: %s', unescape_for_display(location, 'utf-8'))
>          else:
> +            # When started was added, location was only made optional by
> +            # accident.  Matt Nordhoff 20071129
> +            symbol_versioning.warn("As of bzr 0.93 you must pass a location "
> +                                   "to started.", DeprecationWarning,
> +                                   stacklevel=2)
> -             location = ''
> -        self._note('Committing revision %d%s.', revno, location)

I left it because 1.) location is always specified, so it doesn't really
matter, and 2.) to have some feedback that the commit is going okay if
it takes a long time to display the first path changed.

I guess I could go either way, though.
-- 



More information about the bazaar mailing list