Patchwork [V4] commands: 'listening at' line when port specified (issue3976)

login
register
mail settings
Submitter Chinmay Joshi
Date April 8, 2014, 4:17 p.m.
Message ID <CAGwDGc078mdfCT2_-7j2Y9uWZiqYMQf+ARfeOPvCe+WUZNvfTA@mail.gmail.com>
Download mbox | patch
Permalink /patch/4275/
State Superseded
Headers show

Comments

Chinmay Joshi - April 8, 2014, 4:17 p.m.
# HG changeset patch
# User Chinmay Joshi <c@chinmayjoshi.com>
# Date 1396973748 -19800
#      Tue Apr 08 21:45:48 2014 +0530
# Node ID 326b165ed3bd3f6171b24f7ce71ac257d2fc99a3
# Parent  596960a4ad0d342506cf2a601e9736c446a04699
commnads: listening at line when port specified(issue3976)

Now 'listening at ...' line is printed in hg serve when port is
specified. Stays silent for daemon. Changed commnads.py. No test file
needed to be modified.

Mercurial-devel mailing list
Mercurial-devel@selenic.com
http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - April 9, 2014, 7:12 p.m.
On 04/08/2014 06:17 PM, Chinmay Joshi wrote:
> # HG changeset patch
> # User Chinmay Joshi <c@chinmayjoshi.com>
> # Date 1396973748 -19800
> #      Tue Apr 08 21:45:48 2014 +0530
> # Node ID 326b165ed3bd3f6171b24f7ce71ac257d2fc99a3
> # Parent  596960a4ad0d342506cf2a601e9736c446a04699
> commnads: listening at line when port specified(issue3976)

I suggest using a spell checker ("commnads"). Notice also how everybody 
else and the guidelines put spaces around the issue number.

> Now 'listening at ...' line is printed in hg serve when port is
> specified. Stays silent for daemon. Changed commnads.py. No test file
> needed to be modified.

No test changes is a good thing for a refactoring. It is also a good 
thing that a new feature doesn't introduce regressions. But it is bad 
that the new feature don't have any test coverage. Can you modify one of 
the relevant tests so it show the new behaviour? That might require some 
shell script hacking.

> diff -r 596960a4ad0d -r 326b165ed3bd mercurial/commands.py
> --- a/mercurial/commands.py Sat Mar 29 12:18:34 2014 -0500
> +++ b/mercurial/commands.py Tue Apr 08 21:45:48 2014 +0530
> @@ -5250,7 +5250,7 @@
>           util.setsignalhandler()
>           self.httpd = hgweb_server.create_server(self.ui, self.app)
>
> -        if self.opts['port'] and not self.ui.verbose:
> +        if self.opts['port'] and self.opts["daemon"] and not self.ui.verbose:
>               return
>
>           if self.httpd.prefix:
> @@ -5258,7 +5258,11 @@
>           else:
>               prefix = ''
>
> -        port = ':%d' % self.httpd.port
> +        if self.opts['port']:
> +            port = ":%d" % self.opts['port']
> +        else:
> +            port = ':%d' % self.httpd.port

I don't remember if it has been discussed, but as it looks here I wonder 
in which cases httpd.port would be wrong.  If there is good reasons it 
is the way it is, then it might deserve a comment in the code.

/Mads

Patch

diff -r 596960a4ad0d -r 326b165ed3bd mercurial/commands.py
--- a/mercurial/commands.py Sat Mar 29 12:18:34 2014 -0500
+++ b/mercurial/commands.py Tue Apr 08 21:45:48 2014 +0530
@@ -5250,7 +5250,7 @@ 
         util.setsignalhandler()
         self.httpd = hgweb_server.create_server(self.ui, self.app)

-        if self.opts['port'] and not self.ui.verbose:
+        if self.opts['port'] and self.opts["daemon"] and not self.ui.verbose:
             return

         if self.httpd.prefix:
@@ -5258,7 +5258,11 @@ 
         else:
             prefix = ''

-        port = ':%d' % self.httpd.port
+        if self.opts['port']:
+            port = ":%d" % self.opts['port']
+        else:
+            port = ':%d' % self.httpd.port
+
         if port == ':80':
             port = ''
_______________________________________________