Patchwork [3,of,5] paths: use single loop for both search=None|pattern cases

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 9, 2016, 9:45 a.m.
Message ID <2390e7462eec357b2354.1452332743@mimosa>
Download mbox | patch
Permalink /patch/12621/
State Superseded
Commit 7e9dc8bbebf6a0fbfba5bbc83f110aa5673f9f8d
Delegated to: Martin von Zweigbergk
Headers show

Comments

Yuya Nishihara - Jan. 9, 2016, 9:45 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1450011752 -32400
#      Sun Dec 13 22:02:32 2015 +0900
# Node ID 2390e7462eec357b23547d2889717ec98710f9b0
# Parent  d9f3d8bea3ea4d6e501577bc7e3f32c5788c7550
paths: use single loop for both search=None|pattern cases

This will help porting to the formatter API. This patch adds test for empty
pathitems to make sure "hg paths" never say "not found!".
Martin von Zweigbergk - Jan. 12, 2016, 12:37 a.m.
On Sat, Jan 9, 2016 at 1:49 AM Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1450011752 -32400
> #      Sun Dec 13 22:02:32 2015 +0900
> # Node ID 2390e7462eec357b23547d2889717ec98710f9b0
> # Parent  d9f3d8bea3ea4d6e501577bc7e3f32c5788c7550
> paths: use single loop for both search=None|pattern cases
>
> This will help porting to the formatter API. This patch adds test for empty
> pathitems to make sure "hg paths" never say "not found!".
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -5413,18 +5413,16 @@ def paths(ui, repo, search=None):
>      Returns 0 on success.
>      """
>      if search:
> -        for name, path in sorted(ui.paths.iteritems()):
> -            if name == search:
> -                if not ui.quiet:
> -                    ui.write("%s\n" % util.hidepassword(path.rawloc))
> -                return
> -        if not ui.quiet:
> -            ui.warn(_("not found!\n"))
> -        return 1
> +        pathitems = [(name, path) for name, path in ui.paths.iteritems()
> +                     if name == search]
>      else:
>          pathitems = sorted(ui.paths.iteritems())
>
>      for name, path in pathitems:
> +        if search and not ui.quiet:
> +            ui.write("%s\n" % util.hidepassword(path.rawloc))
> +        if search:
> +            continue
>          if ui.quiet:
>              ui.write("%s\n" % name)
>          else:
> @@ -5432,6 +5430,13 @@ def paths(ui, repo, search=None):
>              for subopt, value in sorted(path.suboptions.items()):
>                  ui.write('%s:%s = %s\n' % (name, subopt, value))
>
> +    if search and not pathitems:
> +        if not ui.quiet:
> +            ui.warn(_("not found!\n"))
> +        return 1
>

Why delay this until after the loop over an empty array? I didn't see a
future patch that depended on it either.


> +    else:
> +        return 0
> +
>  @command('phase',
>      [('p', 'public', False, _('set changeset phase to public')),
>       ('d', 'draft', False, _('set changeset phase to draft')),
> diff --git a/tests/test-paths.t b/tests/test-paths.t
> --- a/tests/test-paths.t
> +++ b/tests/test-paths.t
> @@ -3,6 +3,16 @@
>    updating to branch default
>    0 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ cd a
> +
> +with no paths:
> +
> +  $ hg paths
> +  $ hg paths unknown
> +  not found!
> +  [1]
> +
> +with paths:
> +
>    $ echo '[paths]' >> .hg/hgrc
>    $ echo 'dupe = ../b#tip' >> .hg/hgrc
>    $ echo 'expand = $SOMETHING/bar' >> .hg/hgrc
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - Jan. 12, 2016, 11:51 a.m.
On Tue, 12 Jan 2016 00:37:25 +0000, Martin von Zweigbergk wrote:
> On Sat, Jan 9, 2016 at 1:49 AM Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1450011752 -32400
> > #      Sun Dec 13 22:02:32 2015 +0900
> > # Node ID 2390e7462eec357b23547d2889717ec98710f9b0
> > # Parent  d9f3d8bea3ea4d6e501577bc7e3f32c5788c7550
> > paths: use single loop for both search=None|pattern cases
> >
> > This will help porting to the formatter API. This patch adds test for empty
> > pathitems to make sure "hg paths" never say "not found!".
> >
> > diff --git a/mercurial/commands.py b/mercurial/commands.py
> > --- a/mercurial/commands.py
> > +++ b/mercurial/commands.py
> > @@ -5413,18 +5413,16 @@ def paths(ui, repo, search=None):
> >      Returns 0 on success.
> >      """
> >      if search:
> > -        for name, path in sorted(ui.paths.iteritems()):
> > -            if name == search:
> > -                if not ui.quiet:
> > -                    ui.write("%s\n" % util.hidepassword(path.rawloc))
> > -                return
> > -        if not ui.quiet:
> > -            ui.warn(_("not found!\n"))
> > -        return 1
> > +        pathitems = [(name, path) for name, path in ui.paths.iteritems()
> > +                     if name == search]
> >      else:
> >          pathitems = sorted(ui.paths.iteritems())
> >
> >      for name, path in pathitems:
> > +        if search and not ui.quiet:
> > +            ui.write("%s\n" % util.hidepassword(path.rawloc))
> > +        if search:
> > +            continue
> >          if ui.quiet:
> >              ui.write("%s\n" % name)
> >          else:
> > @@ -5432,6 +5430,13 @@ def paths(ui, repo, search=None):
> >              for subopt, value in sorted(path.suboptions.items()):
> >                  ui.write('%s:%s = %s\n' % (name, subopt, value))
> >
> > +    if search and not pathitems:
> > +        if not ui.quiet:
> > +            ui.warn(_("not found!\n"))
> > +        return 1
> >
> 
> Why delay this until after the loop over an empty array? I didn't see a
> future patch that depended on it either.

fm.end() is necessary to print "[]" by "hg paths -Tjson unknown_name".
Martin von Zweigbergk - Jan. 12, 2016, 5:03 p.m.
On Tue, Jan 12, 2016 at 3:51 AM Yuya Nishihara <yuya@tcha.org> wrote:

> On Tue, 12 Jan 2016 00:37:25 +0000, Martin von Zweigbergk wrote:
> > On Sat, Jan 9, 2016 at 1:49 AM Yuya Nishihara <yuya@tcha.org> wrote:
> >
> > > # HG changeset patch
> > > # User Yuya Nishihara <yuya@tcha.org>
> > > # Date 1450011752 -32400
> > > #      Sun Dec 13 22:02:32 2015 +0900
> > > # Node ID 2390e7462eec357b23547d2889717ec98710f9b0
> > > # Parent  d9f3d8bea3ea4d6e501577bc7e3f32c5788c7550
> > > paths: use single loop for both search=None|pattern cases
> > >
> > > This will help porting to the formatter API. This patch adds test for
> empty
> > > pathitems to make sure "hg paths" never say "not found!".
> > >
> > > diff --git a/mercurial/commands.py b/mercurial/commands.py
> > > --- a/mercurial/commands.py
> > > +++ b/mercurial/commands.py
> > > @@ -5413,18 +5413,16 @@ def paths(ui, repo, search=None):
> > >      Returns 0 on success.
> > >      """
> > >      if search:
> > > -        for name, path in sorted(ui.paths.iteritems()):
> > > -            if name == search:
> > > -                if not ui.quiet:
> > > -                    ui.write("%s\n" % util.hidepassword(path.rawloc))
> > > -                return
> > > -        if not ui.quiet:
> > > -            ui.warn(_("not found!\n"))
> > > -        return 1
> > > +        pathitems = [(name, path) for name, path in
> ui.paths.iteritems()
> > > +                     if name == search]
> > >      else:
> > >          pathitems = sorted(ui.paths.iteritems())
> > >
> > >      for name, path in pathitems:
> > > +        if search and not ui.quiet:
> > > +            ui.write("%s\n" % util.hidepassword(path.rawloc))
> > > +        if search:
> > > +            continue
> > >          if ui.quiet:
> > >              ui.write("%s\n" % name)
> > >          else:
> > > @@ -5432,6 +5430,13 @@ def paths(ui, repo, search=None):
> > >              for subopt, value in sorted(path.suboptions.items()):
> > >                  ui.write('%s:%s = %s\n' % (name, subopt, value))
> > >
> > > +    if search and not pathitems:
> > > +        if not ui.quiet:
> > > +            ui.warn(_("not found!\n"))
> > > +        return 1
> > >
> >
> > Why delay this until after the loop over an empty array? I didn't see a
> > future patch that depended on it either.
>
> fm.end() is necessary to print "[]" by "hg paths -Tjson unknown_name".
>

In that case, looks good. Pushed to the clowncopter, thanks.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5413,18 +5413,16 @@  def paths(ui, repo, search=None):
     Returns 0 on success.
     """
     if search:
-        for name, path in sorted(ui.paths.iteritems()):
-            if name == search:
-                if not ui.quiet:
-                    ui.write("%s\n" % util.hidepassword(path.rawloc))
-                return
-        if not ui.quiet:
-            ui.warn(_("not found!\n"))
-        return 1
+        pathitems = [(name, path) for name, path in ui.paths.iteritems()
+                     if name == search]
     else:
         pathitems = sorted(ui.paths.iteritems())
 
     for name, path in pathitems:
+        if search and not ui.quiet:
+            ui.write("%s\n" % util.hidepassword(path.rawloc))
+        if search:
+            continue
         if ui.quiet:
             ui.write("%s\n" % name)
         else:
@@ -5432,6 +5430,13 @@  def paths(ui, repo, search=None):
             for subopt, value in sorted(path.suboptions.items()):
                 ui.write('%s:%s = %s\n' % (name, subopt, value))
 
+    if search and not pathitems:
+        if not ui.quiet:
+            ui.warn(_("not found!\n"))
+        return 1
+    else:
+        return 0
+
 @command('phase',
     [('p', 'public', False, _('set changeset phase to public')),
      ('d', 'draft', False, _('set changeset phase to draft')),
diff --git a/tests/test-paths.t b/tests/test-paths.t
--- a/tests/test-paths.t
+++ b/tests/test-paths.t
@@ -3,6 +3,16 @@ 
   updating to branch default
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cd a
+
+with no paths:
+
+  $ hg paths
+  $ hg paths unknown
+  not found!
+  [1]
+
+with paths:
+
   $ echo '[paths]' >> .hg/hgrc
   $ echo 'dupe = ../b#tip' >> .hg/hgrc
   $ echo 'expand = $SOMETHING/bar' >> .hg/hgrc