[PATCH] bzr whoami

[Martin Pool]

On 28 Jun 2006, Robey Pointer <robey at lag.net> wrote:
> I spent some time today fixing up my ancient 'whoami' patch to fit  
> the current bzr.dev trunk.  The patch adds:
> 
> 	* bzr whoami <identity> -- to set your global id
> 	* bzr whoami --branch <identity> -- to set your id just on this 
> 	branch
> 	* unicode-safe identity

I'd be happy to have that come in.

> +class TestWhoami(ExternalBase):
> +
> +    def test_whoami(self):
> +        # this should always identify something, if only "john at localhost"
> +        self.run_bzr("whoami")
> +        self.run_bzr("whoami", "--email")

You don't make any assertions about the result, other than that the
command succeeds.  Is it intentional?


> +
> +        self.assertEquals(self.run_bzr("whoami", "--email")[0].count('@'), 1)
> +        
> +    def test_whoami_branch(self):
> +        """branch specific user identity works."""
> +        self.run_bzr('init')
> +        b = bzrlib.branch.Branch.open('.')
> +        b.get_config().set_user_option('email', 'Branch Identity <branch at identi.ty>')
> +        bzr_email = os.environ.get('BZREMAIL')
> +        if bzr_email is not None:
> +            del os.environ['BZREMAIL']
> +        whoami = self.run_bzr("whoami")[0]
> +        whoami_email = self.run_bzr("whoami", "--email")[0]
> +        self.assertTrue(whoami.startswith('Branch Identity <branch at identi.ty>'))
> +        self.assertTrue(whoami_email.startswith('branch at identi.ty'))
> +
> +        # Verify that the environment variable overrides the value 
> +        # in the file
> +        os.environ['BZREMAIL'] = 'Different ID <other at environ.ment>'
> +        whoami = self.run_bzr("whoami")[0]
> +        whoami_email = self.run_bzr("whoami", "--email")[0]
> +        self.assertTrue(whoami.startswith('Different ID <other at environ.ment>'))
> +        self.assertTrue(whoami_email.startswith('other at environ.ment'))
> +        if bzr_email is not None:
> +            os.environ['BZREMAIL'] = bzr_email

The save/restore as a matter of form should be in a try/finally block,
if only to help the reader.  (I see this is not your new code but please
fix it while you're there.)

>  class cmd_whoami(Command):
> -    """Show bzr user id."""
> -    takes_options = ['email']
> +    """Show or set bzr user id."""
> +    takes_options = [ Option('email',
> +                             help='display email address only'),
> +                      Option('branch',
> +                             help='set identity for the current branch instead of '
> +                                  'gobally'),
> +                    ]
> +    takes_args = ['name?']
> +    encoding_type = 'replace'

The docstring should probably give examples of how it can be used to set
the identity.

>      
>      @display_command
> -    def run(self, email=False):
> -        try:
> -            c = WorkingTree.open_containing(u'.')[0].branch.get_config()
> -        except NotBranchError:
> -            c = config.GlobalConfig()
> +    def run(self, email=False, branch=False, name=None):
> +        if name is None:
> +            # use branch if we're inside one; otherwise global config
> +            try:
> +                c = WorkingTree.open_containing(u'.')[0].branch.get_config()
> +            except NotBranchError:
> +                c = config.GlobalConfig()
> +        else:
> +            # why is this necessary?
> +            name = name.decode(bzrlib.user_encoding)
> +            # use global config unless --branch given
> +            if branch:
> +                c = WorkingTree.open_containing(u'.')[0].branch.get_config()
> +            else:
> +                c = config.GlobalConfig()
> +            c.set_user_option('email', name)
> +
>          if email:
> -            print c.user_email()
> +            self.outf.write(c.user_email())
>          else:
> -            print c.username()
> +            self.outf.write(c.username())
> +        self.outf.write('\n')

This seems to still print the username when you set it - is that really
best?  I'd expect either silence, or a confirmation message.

I'm not sure why the decode is necessary - maybe sys.argv is byte
strings.  But should we perhaps decode them at a higher level?

Could you please add a note to NEWS, and please update the user
documentation to explain that the userid can be set this way.


-- 
Martin