[PATCH 1 of 2] minirst: Support for admonitions

Martin Geisler mg at aragost.com
Thu Sep 23 03:27:59 CDT 2010


Erik Zielke <ez at aragost.com> writes:

> # HG changeset patch
> # User Erik Zielke <erikzielke at hotmail.com>
> # Date 1285163519 -7200
> # Node ID ded8e44b92bda9f6cb6b12c49e3d36b19c47e37d
> # Parent  8a5b6383ba02c053eac7a10a26502edd35355a90
> minirst: Support for admonitions

Let me add that this was something that the old asciidoc format
supported and that is actually why we have 'NOTE:' scattered here and
there in the help strings:

  http://www.methods.co.nz/asciidoc/userguide.html#X28

I've wanted to support the ReST equivalents for a long time. We already
use them in our HTML docs as you can see here:

  http://www.selenic.com/mercurial/hgrc.5.html#alias

> diff -r 8a5b6383ba02 -r ded8e44b92bd mercurial/minirst.py
> --- a/mercurial/minirst.py	Tue Sep 21 23:58:32 2010 +0200
> +++ b/mercurial/minirst.py	Wed Sep 22 15:51:59 2010 +0200
> @@ -24,6 +24,8 @@
>  
>  - definition lists
>  
> +- admonitions
> +
>  - bullet lists (items must start with '-')
>  
>  - enumerated lists (no autonumbering)
> @@ -37,6 +39,8 @@
>  
>  import re, sys
>  import util, encoding
> +from i18n import _
> +
>  
>  def replace(text, substs):
>      utext = text.decode(encoding.encoding)
> @@ -292,12 +296,66 @@
>              i += 2
>      return blocks
>  
> +def addadmonitions(blocks):

Perhaps it should be called findadmonitions?

> +    """    
> +    Makes the type of the block an admonition block if 
> +    the first line is an admonition directive 
> +    """

You have trailing whitespace in the above tree lines (4 spaces after
""", 1 space after each of the following lines). If you enable the color
extension, then 'hg diff' will highlight such trailing whitespace :)


> +    
> +    i = 0
> +    
> +    admonitiongroup1 = 'ADMONITION|ATTENTION|CAUTION|DANGER|ERROR|HINT|'
> +    admonitiongroup2 = 'IMPORTANT|NOTE|TIP|WARNING|ADMONITION'
> +    pattern = '\.\. ('+ admonitiongroup1 + admonitiongroup2 +')::'

I can see you're missing the Python syntax for long strings:

  pattern = (r"\.\. (admonition|attention|caution|danger|error|hint|"
             r"important|note|tip|warning|admonition)::")

Here I used parenthesis to allow me to split the string on two lines and
used the fact that string literals are joined together automatically. I
also used a "raw string" in which \ is no longer special. So

  r"\." == "\\."

Now, it happens to be the case that your original "\." == "\\.", but
that is only because \. is not a legal escape sequence like \n or \t. I
always avoid relying on that behavior since I find it confusing.

Finally, since the case is ignored in directive names, I opted to make
them lowercase already here to minimize the shouting :)

> +    prog = re.compile(pattern, flags=re.IGNORECASE)
> +    while i < len(blocks):
> +        if(prog.match(blocks[i]['lines'][0])):
> +            endpos = blocks[i]['lines'][0].find('::')

I'm pretty sure the match object you get from prog.match can tell you
the end position. Other place do it like this:

  m = admonitionre.match(...)
  if m:
    # do something with m

> +            blocks[i]['type'] = 'admonition'
> +            admonitiontitle = blocks[i]['lines'][0][3:endpos].lower()
> +            
> +            if admonitiontitle == 'admonition':
> +                line = blocks[i]['lines'][0]
> +                admonitiontitle = line[endpos + 3:].lower()

I'm not sure what this does -- please include a comment that describes
this detail of The ReST syntax.

> +                
> +            blocks[i]['admonitiontitle'] = admonitiontitle
> +            del blocks[i]['lines'][0]

Ah, right, we talked a bit about this face to face... you only support
the case where the title is put on a line by itself. Like this:

  .. note::
     Remember to brush your teeth.

I don't think it would be difficult to support

  .. note:: Remember to brush your teeth.

and declare that the indentation of the '..' is what counts. That would
give us blocks indented 3 spaces by default -- that should be okay. It's
a bit different from what we're used to and if we reallly want, I'm sure
we could normalize it to 4 spaces.

> +        i = i + 1
> +    return blocks
>  
>  def formatblock(block, width):
>      """Format a block according to width."""
>      if width <= 0:
>          width = 78
>      indent = ' ' * block['indent']
> +    if block['type'] == 'admonition':
> +        admonitionformats = {'attention': _('Attention:'), 

Trailing whitespace. I would just calls the dict 'titles'.

> +                              'caution': _('Caution:'),
> +                              'danger': _('!Danger!')  ,
> +                              'error': _('Error:'),
> +                              'hint': _('Hint:'),
> +                              'important': _('Important:'),
> +                              'note': _('Note'':'),
> +                              'tip': _('Tip:'),
> +                              'warning': _('Warning!')}
> +         
> +        format = '%s:'
> +         
> +        if block['admonitiontitle'] in admonitionformats.keys():

No need for .keys() here.

> +            format = admonitionformats[(block['admonitiontitle'])]
> +            admonition =  format
> +        else:
> +            admonition = (format % block['admonitiontitle']).title()

I'm not sure we should change the case of the text -- this is a
translated piece of text, and other languages does not follow the
English way of titlecasing headings.

So let us just use the title unchanged and let people add a ':' or '!'
as needed. I would even be fine with not supporting custom titles at
all.

> +
> +             
> +        hang = len(block['lines'][-1]) - len(block['lines'][-1].lstrip())
> +        defindent = indent + hang * ' '
> +        text = ' '.join(map(str.strip, block['lines']))
> +        return '%s\n%s' % (indent + admonition, util.wrap(text, width=width,
> +                                           initindent=defindent,
> +                                           hangindent=defindent))
>      if block['type'] == 'margin':
>          return ''
>      if block['type'] == 'literal':
> @@ -363,6 +421,7 @@
>      blocks = splitparagraphs(blocks)
>      blocks = updatefieldlists(blocks)
>      blocks = addmargins(blocks)
> +    blocks = addadmonitions(blocks)
>      text = '\n'.join(formatblock(b, width) for b in blocks)
>      if keep is None:
>          return text
> @@ -389,4 +448,5 @@
>      blocks = debug(updatefieldlists, blocks)
>      blocks = debug(findsections, blocks)
>      blocks = debug(addmargins, blocks)
> +    blocks = debug(addadmonitions, blocks)
>      print '\n'.join(formatblock(b, 30) for b in blocks)
> diff -r 8a5b6383ba02 -r ded8e44b92bd tests/test-minirst.py
> --- a/tests/test-minirst.py	Tue Sep 21 23:58:32 2010 +0200
> +++ b/tests/test-minirst.py	Wed Sep 22 15:51:59 2010 +0200
> @@ -197,3 +197,21 @@
>  ------------------------------
>  """
>  debugformat('sections', sections, 20)
> +
> +
> +admonitions = """
> +.. NOTE::
> +   This is a note
> +
> +   - Bullet 1
> +   - Bullet 2
> +   - BUllet 3

Let's just drop the third BUllet :)

> +
> +   .. ADMONITION:: General note
> +      Test general note
> +
> +.. DANGER::
> +   This is danger
> +"""
> +
> +debugformat('admonitions', admonitions, 30)
> diff -r 8a5b6383ba02 -r ded8e44b92bd tests/test-minirst.py.out
> --- a/tests/test-minirst.py.out	Tue Sep 21 23:58:32 2010 +0200
> +++ b/tests/test-minirst.py.out	Wed Sep 22 15:51:59 2010 +0200
> @@ -318,3 +318,19 @@
>  ---------------------------
>  ----------------------------------------------------------------------
>  
> +admonitions formatted to fit within 30 characters:
> +----------------------------------------------------------------------
> +Note:
> +   This is a note
> +
> +   - Bullet 1
> +   - Bullet 2
> +   - BUllet 3
> +
> +   General Note:
> +      Test general note
> +
> +!Danger!
> +   This is danger
> +----------------------------------------------------------------------


-- 
Martin Geisler

aragost Trifork
Professional Mercurial support
http://aragost.com/mercurial/


More information about the Mercurial-devel mailing list