[MERGE] LockDir.lock_write() shouldn't require an email address

[Martin Pool]

On 30/09/2006, at 06:26 , John Arbash Meinel wrote:

> The attached patch fixes a small bug in LockDir.
>
> Basically, it requires a semi-valid email address to be configured, or
> '.lock_write()' will fail.
>
> Also, extract_email_address() shouldn't be raising a generic BzrError.
> So this patch updates it to raise a better exception, and then has the
> callers catch the right exception.
>
> Because of inheritance, we don't need to do a deprecation phase,  
> because
> NoEmailInUsername is a child of BzrError.

+1

>
>  def annotate_file(branch, rev_id, file_id, verbose=False, full=False,
> @@ -81,6 +81,6 @@
>              author = rev.committer
>              try:
>                  author = extract_email_address(author)
> -            except BzrError:
> +            except NoEmailInUsername:
>                  pass        # use the whole name
>          yield (revno_str, author, date_str, origin, text)

well done - it would actually be worth grepping for catches of  
BzrError, they're unlikely to be right.
>

> === modified file bzrlib/builtins.py
> --- bzrlib/builtins.py
> +++ bzrlib/builtins.py
> @@ -1909,7 +1909,7 @@
>          # display a warning if an email address isn't included in  
> the given name.
>          try:
>              config.extract_email_address(name)
> -        except BzrError, e:
> +        except errors.NoEmailInUsername, e:
>              warning('"%s" does not seem to contain an email  
> address.  '
>                      'This is allowed, but not recommended.', name)

OK.

This is not really an ideal message - imagine if it appears on stderr:

   "" does not seem to contain an email address...

I think it's better if messages give some context about *where* the  
bad value comes from, or why.

(I realise you didn't add the message, you don't have to fix it now.)

-- 
Martin