Patchwork templater: show the style list when I try to use a wrong one

login
register
mail settings
Submitter Iulian Stana
Date April 18, 2013, 7:23 p.m.
Message ID <4d5818e25c6da500f3d2.1366313013@doppler>
Download mbox | patch
Permalink /patch/1433/
State Superseded, archived
Commit 6ba6e345961e9844c0f97247a091a2daeddf5ba2
Headers show

Comments

Iulian Stana - April 18, 2013, 7:23 p.m.
# HG changeset patch
# User Iulian Stana <julian.stana@gmail.com>
# Date 1366312898 -10800
#      Thu Apr 18 22:21:38 2013 +0300
# Node ID 4d5818e25c6da500f3d2d67ffce47ea9662aa166
# Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
templater: show the style list when I try to use a wrong one

When someone try to use a wrong style, a list with sugestions will appear.

In the test-log.t file it's a test that prove this thing.
Simon King - April 19, 2013, 9:22 a.m.
On Thu, Apr 18, 2013 at 8:23 PM, Iulian Stana <julian.stana@gmail.com> wrote:
> # HG changeset patch
> # User Iulian Stana <julian.stana@gmail.com>
> # Date 1366312898 -10800
> #      Thu Apr 18 22:21:38 2013 +0300
> # Node ID 4d5818e25c6da500f3d2d67ffce47ea9662aa166
> # Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
> templater: show the style list when I try to use a wrong one
>
> When someone try to use a wrong style, a list with sugestions will appear.
>
> In the test-log.t file it's a test that prove this thing.
>
> diff -r 7d31f2e42a8a -r 4d5818e25c6d mercurial/templater.py
> --- a/mercurial/templater.py    Mon Apr 15 18:57:04 2013 -0300
> +++ b/mercurial/templater.py    Thu Apr 18 22:21:38 2013 +0300
> @@ -392,6 +392,21 @@
>
>  engines = {'default': engine}
>
> +def stylelist():
> +    path = templatepath()[0]
> +    dirlist =  os.listdir(path)
> +    stylelist = ''
> +    notfirst = False
> +    for file in dirlist:
> +        split = file.split(".")
> +        if split[0] == "map-cmdline":
> +            if notfirst == True:
> +                stylelist = stylelist + ','
> +            else:
> +                notfirst = True
> +            stylelist = stylelist + ' ' + split[1]
> +    return stylelist
> +

There are a few python idioms that could help you here. One is the
"enumerate" function, which yields pairs of (index, item) tuples from
a sequence. You could use it like this:

for index, file in enumerate(dirlist):
    split = file.split(".")
    if split[0] == "map-cmdline":
        if index == 0:
            stylelist = stylelist + ','
    # etc.

Also, in Python, it's generally not considered good practice to build
strings by repeatedly adding lots of small strings. Instead it is
normally better to build a list, and then use the str.join method to
concatenate all the strings. This also has the benefit of not having
to treat the first item differently:

stylelist = []
for file in dirlist:
    split = file.split(".")
    if split[0] == "map-cmdline":
        stylelist.append(split[0])

return ", ".join(stylelist)

Finally, purely as a personal preference, I would have the stylelist()
function return a list rather than a string, and do the joining with
the comma in the calling function.

>  class templater(object):
>
>      def __init__(self, mapfile, filters={}, defaults={}, cache={},
> @@ -413,7 +428,7 @@
>          if not mapfile:
>              return
>          if not os.path.exists(mapfile):
> -            raise util.Abort(_('style not found: %s') % mapfile)
> +            raise util.Abort(_("style '%s' not found \n(available styles:%s)") % (mapfile,stylelist()))
>

I think what Kevin was trying to get you to do here was something like:

    raise util.Abort(_("style '%s' not found") % mapfile,
                     hint=_("available styles: %s") % stylelist())

Hope that helps,

Simon

>          conf = config.config()
>          conf.read(mapfile)
> diff -r 7d31f2e42a8a -r 4d5818e25c6d tests/test-log.t
> --- a/tests/test-log.t  Mon Apr 15 18:57:04 2013 -0300
> +++ b/tests/test-log.t  Thu Apr 18 22:21:38 2013 +0300
> @@ -84,6 +84,14 @@
>    abort: cannot follow file not in parent revision: "dir"
>    [255]
>
> +-f, a wrong style
> +
> +  $ hg log -f -l1 --style something
> +  abort: style 'something' not found
> +  (available styles: changelog, bisect, default, xml, compact)
> +  [255]
> +
> +
>  -f, but no args
>
>    $ hg log -f
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff -r 7d31f2e42a8a -r 4d5818e25c6d mercurial/templater.py
--- a/mercurial/templater.py	Mon Apr 15 18:57:04 2013 -0300
+++ b/mercurial/templater.py	Thu Apr 18 22:21:38 2013 +0300
@@ -392,6 +392,21 @@ 
 
 engines = {'default': engine}
 
+def stylelist():
+    path = templatepath()[0]
+    dirlist =  os.listdir(path)
+    stylelist = ''
+    notfirst = False
+    for file in dirlist:
+        split = file.split(".")
+        if split[0] == "map-cmdline":
+            if notfirst == True:
+                stylelist = stylelist + ','
+            else:
+                notfirst = True
+            stylelist = stylelist + ' ' + split[1]
+    return stylelist
+
 class templater(object):
 
     def __init__(self, mapfile, filters={}, defaults={}, cache={},
@@ -413,7 +428,7 @@ 
         if not mapfile:
             return
         if not os.path.exists(mapfile):
-            raise util.Abort(_('style not found: %s') % mapfile)
+            raise util.Abort(_("style '%s' not found \n(available styles:%s)") % (mapfile,stylelist()))
 
         conf = config.config()
         conf.read(mapfile)
diff -r 7d31f2e42a8a -r 4d5818e25c6d tests/test-log.t
--- a/tests/test-log.t	Mon Apr 15 18:57:04 2013 -0300
+++ b/tests/test-log.t	Thu Apr 18 22:21:38 2013 +0300
@@ -84,6 +84,14 @@ 
   abort: cannot follow file not in parent revision: "dir"
   [255]
 
+-f, a wrong style
+
+  $ hg log -f -l1 --style something
+  abort: style 'something' not found 
+  (available styles: changelog, bisect, default, xml, compact)
+  [255]
+
+
 -f, but no args
 
   $ hg log -f