Merge lp://qastaging/~jelmer/bzr/cmd-branches into lp://qastaging/bzr

Why self.outf.write rather than trace.note() ?

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/17/2011 1:49 PM, Jonathan Riddell wrote:
> Review: Approve Why self.outf.write rather than trace.note() ?
>

trace.note() is conceptually meta-information (it goes to stderr).
self.outf.write() goes to stdout and implies it is the data you actually
want.

Consider scripting:

for b in $(bzr branches); do something_to $b; done

You certainly want the branch names to go to stdout, but something like
progress bars could still go to stderr.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5LrKAACgkQJdeBCYSNAANx7wCgq2JlL/r7n6AGKU+rcs4UBKr7
QFAAoKRHnCTGRUQ2opOY0KI7qFw7ZMD5
=6dHu
-----END PGP SIGNATURE-----

sent to pqm by email

On 8/17/2011 11:38 AM, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/cmd-branches into
lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #826820 in Bazaar: ""bzr branches" should list colocated branches if
supported"
> https://bugs.launchpad.net/bzr/+bug/826820
>
> For more details, see:
> https://code.launchpad.net/~jelmer/bzr/cmd-branches/+merge/71577
>
> Add a 'bzr branches' command, which lists all of the colocated branches in
a location.
>
> This is different from the 'bzr branches' command that is currently
present in bzrtools.

I'm a little hesitant to add something that is incompatible with what
users might expect with the same name.

Also, we've had requests in the past that "find_branches()" be an
iterator, so that things like qbzr can start displaying without
blocking. So calling it "list_branches()" seems to force it to be a list.

Though if the function already exists in core, then that's ok.

+ self.outf.write(" %s\n" % branch.name.encode(self.outf.encoding))

That's a UnicodeEncodeError if the branch name can't be represented in the local encoding. There are no tests for non-ascii branch names, so maybe that's not a case you're concerned about. Otherwise, you'd need something along the lines urlutils.unescape_for_display (which has its own problems) or similar means for escaping problematic characters.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>> Also, we've had requests in the past that "find_branches()" be an
>> iterator, so that things like qbzr can start displaying without
>> blocking. So calling it "list_branches()" seems to force it to be
>> a list.
>
>> Though if the function already exists in core, then that's ok.
> list_branches() will yield all branches in the bzrdir it's called
> on.
>
> BzrDir.find_branches() is a class method that will recursively look
> for branches under the path in which the BzrDir lives.
>
> Cheers,
>
> Jelmer
>
>
Sure, but
+ dir = bzrdir.BzrDir.open_containing(location)[0]
+ for branch in dir.list_branches():

Is using "list_branches" not "find_branches".

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5L0q8ACgkQJdeBCYSNAAPAhQCfUfcJdKIvy/JcjY8ksfF+1/r/
njsAn3/qxoisfp0ipUpyN2+8ut1UZ2yM
=xTlp
-----END PGP SIGNATURE-----