Patchwork revsets: added authuri when login information is requested (issue3209)

login
register
mail settings
Submitter Lucas Moscovicz
Date Jan. 16, 2014, 12:54 a.m.
Message ID <c7b43e384d85c17c7e90.1389833649@devrs047.prn2.facebook.com>
Download mbox | patch
Permalink /patch/3338/
State Changes Requested
Headers show

Comments

Lucas Moscovicz - Jan. 16, 2014, 12:54 a.m.
# HG changeset patch
# User Lucas Moscovicz <lmoscovicz@fb.com>
# Date 1389833180 28800
#      Wed Jan 15 16:46:20 2014 -0800
# Node ID c7b43e384d85c17c7e90326ff9c4f6d5d907c196
# Parent  f694cd81b600b65d23dcdc7a02cfd6a57dd1d018
revsets: added authuri when login information is requested (issue3209)

When users are using a revset they can get multiple password prompts.
This prompts have no extra information about which password is being requested
so I added the authuri to the prompt to make it recognizable.
Mads Kiilerich - Jan. 16, 2014, 11:47 a.m.
On 01/16/2014 01:54 AM, Lucas Moscovicz wrote:
> # HG changeset patch
> # User Lucas Moscovicz <lmoscovicz@fb.com>
> # Date 1389833180 28800
> #      Wed Jan 15 16:46:20 2014 -0800
> # Node ID c7b43e384d85c17c7e90326ff9c4f6d5d907c196
> # Parent  f694cd81b600b65d23dcdc7a02cfd6a57dd1d018
> revsets: added authuri when login information is requested (issue3209)

It is not clear from the description or the patch what it has to do with 
revsets. Reading the bug makes it more clear. I would file it under 
"http" or "url" instead of "revsets". Revsets is just one of the ways to 
show this "issue".

> When users are using a revset they can get multiple password prompts.
> This prompts have no extra information about which password is being requested
> so I added the authuri to the prompt to make it recognizable.

What to the prompt look like with this patch? An example would be 
helpful (especially when there is no test).

Could you add some test coverage of this? It is almost covered by 
test-http.t. Forcing it to interactive mode could probably make it reach 
the line you are touching.

> diff --git a/mercurial/url.py b/mercurial/url.py
> --- a/mercurial/url.py
> +++ b/mercurial/url.py
> @@ -35,7 +35,8 @@
>               if not self.ui.interactive():
>                   raise util.Abort(_('http authorization required'))

This message should get the same treatment.

>   
> -            self.ui.write(_("http authorization required\n"))
> +            self.ui.write(_("http authorization required for %s\n") %
> +                          urllib.splitquery(authuri)[0])

It should probably also use util.hidepassword ... just in case.

urllib.splitquery is not documented on 
http://docs.python.org/2/library/urllib.html and the docstring do not 
say much on how it works on urls. Based on experience with other things 
realted to url/http, I wonder if the function is available and stable in 
all relevant Python versions.

/Mads

/Mads

Patch

diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -35,7 +35,8 @@ 
             if not self.ui.interactive():
                 raise util.Abort(_('http authorization required'))
 
-            self.ui.write(_("http authorization required\n"))
+            self.ui.write(_("http authorization required for %s\n") %
+                          urllib.splitquery(authuri)[0])
             self.ui.write(_("realm: %s\n") % realm)
             if user:
                 self.ui.write(_("user: %s\n") % user)