[PATCH] bzr whoami

[Robey Pointer]

On 1 Jul 2006, at 19:43, John Arbash Meinel wrote:

> ^-- Three comments on the copyright statement.

Rather than responding line by line, I'll just say that all of these  
comments are referring to the original code.  I corrected them and  
submitted them.  But now to avoid 20 more rounds of this, I'm going  
to ask for an up/down vote as-is.


> You only need to set the 'encoding' parameter if you want to force the
> sys.stdout() encoding.

That's actually the point.  The tests verify that 'whoami' works for  
at least those two encodings, using an identity that can't be  
expressed in ascii.


> So one thing we need to decide is if 'whoami' should fail if it cannot
> print your name properly in the current encoding. I'm a little  
> torn. On
> one hand I would like to be strict, because a shell front end might  
> use
> whomai.
> On the other, it might confuse users if they get an encoding error  
> while
> trying to just check if they set their username correctly.

I don't have a strong opinion on this either.


> v- in general, I prefer 'run_bzr' to runbzr. I personally think runbzr
> should go away. (It only exists on ExternalBase, not  
> TestCaseInTempDir).

I just missed that one when migrating the rest.  I do think that it's  
utterly confusing which one of the various run*bzr* methods is meant  
to be called.  Why not mark the other ones as deprecated?  I picked  
'run_bzr' pretty much by personal choice since I couldn't remember  
which one was preferred from the last time the 'whoami' patch failed  
to land.


> Something doesn't seem right here. I really don't like the look of
> WorkingTree.open_containing('.')[0].branch.get_config()
>
> Also, because whoami is a branch config, it should still work even
> without a working tree. So I would rather see:

Done.


> I think we may want to validate that the supplied value really is a:
>
> "Full Name <email at address.com>"
>
> Rather than just letting them do:
>
> bzr whoami george
>
> We do accept short names (I don't think we explicitly require email
> addresses anywhere). But we would probably at least want to warn the
> user that it is recommended to use a full email address, etc.

I think that's a good idea.  I would want to just reuse the method  
that does this extraction in config, so it should be a separate patch.

robey