Patchwork [3,of,3] minirst: remove redundant _admonitions set

login
register
mail settings
Submitter Gregory Szorc
Date March 30, 2017, 3:19 a.m.
Message ID <191b0b4697cc979464ec.1490843982@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/19840/
State Accepted
Headers show

Comments

Gregory Szorc - March 30, 2017, 3:19 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1490843966 25200
#      Wed Mar 29 20:19:26 2017 -0700
# Node ID 191b0b4697cc979464ec5ad9f04ef1ffa0f2a6cb
# Parent  c4329b811738046b70661f5647df50d7d28b2362
minirst: remove redundant _admonitions set

As Yuya pointed out during a review a month ago, _admonitions and
_admonitiontitles are largely redundant. With the last commit, they
are exactly redundant. So, remove _admonitions and use
_admonitiontitles.keys() instead.
Ryan McElroy - March 30, 2017, 8:38 a.m.
On 3/30/17 4:19 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1490843966 25200
> #      Wed Mar 29 20:19:26 2017 -0700
> # Node ID 191b0b4697cc979464ec5ad9f04ef1ffa0f2a6cb
> # Parent  c4329b811738046b70661f5647df50d7d28b2362
> minirst: remove redundant _admonitions set

This series looks good to me and is a nice cleanup. Marked as 
pre-reviewed in patchwork.

>
> As Yuya pointed out during a review a month ago, _admonitions and
> _admonitiontitles are largely redundant. With the last commit, they
> are exactly redundant. So, remove _admonitions and use
> _admonitiontitles.keys() instead.
>
> diff --git a/mercurial/minirst.py b/mercurial/minirst.py
> --- a/mercurial/minirst.py
> +++ b/mercurial/minirst.py
> @@ -413,24 +413,12 @@ def prunecomments(blocks):
>       return blocks
>   
>   
> -_admonitions = set([
> -    'attention',
> -    'caution',
> -    'danger',
> -    'error',
> -    'hint',
> -    'important',
> -    'note',
> -    'tip',
> -    'warning',
> -])
> -
>   def findadmonitions(blocks, admonitions=None):
>       """
>       Makes the type of the block an admonition block if
>       the first line is an admonition directive
>       """
> -    admonitions = admonitions or _admonitions
> +    admonitions = admonitions or _admonitiontitles.keys()
>   
>       admonitionre = re.compile(br'\.\. (%s)::' % '|'.join(sorted(admonitions)),
>                                 flags=re.IGNORECASE)
>
Yuya Nishihara - March 30, 2017, 2:15 p.m.
On Thu, 30 Mar 2017 09:38:31 +0100, Ryan McElroy wrote:
> On 3/30/17 4:19 AM, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1490843966 25200
> > #      Wed Mar 29 20:19:26 2017 -0700
> > # Node ID 191b0b4697cc979464ec5ad9f04ef1ffa0f2a6cb
> > # Parent  c4329b811738046b70661f5647df50d7d28b2362
> > minirst: remove redundant _admonitions set
> 
> This series looks good to me and is a nice cleanup. Marked as 
> pre-reviewed in patchwork.

Queued, many thanks.

Patch

diff --git a/mercurial/minirst.py b/mercurial/minirst.py
--- a/mercurial/minirst.py
+++ b/mercurial/minirst.py
@@ -413,24 +413,12 @@  def prunecomments(blocks):
     return blocks
 
 
-_admonitions = set([
-    'attention',
-    'caution',
-    'danger',
-    'error',
-    'hint',
-    'important',
-    'note',
-    'tip',
-    'warning',
-])
-
 def findadmonitions(blocks, admonitions=None):
     """
     Makes the type of the block an admonition block if
     the first line is an admonition directive
     """
-    admonitions = admonitions or _admonitions
+    admonitions = admonitions or _admonitiontitles.keys()
 
     admonitionre = re.compile(br'\.\. (%s)::' % '|'.join(sorted(admonitions)),
                               flags=re.IGNORECASE)