Patchwork [v2] releasenotes: improve parsing around bullet points

login
register
mail settings
Submitter Rishabh Madan
Date June 13, 2017, 8:38 p.m.
Message ID <15c645861efc09cf79cf.1497386301@bunty>
Download mbox | patch
Permalink /patch/21361/
State Superseded
Headers show

Comments

Rishabh Madan - June 13, 2017, 8:38 p.m.
# HG changeset patch
# User Rishabh Madan <rishabhmadan96@gmail.com>
# Date 1497386167 -7200
#      Tue Jun 13 22:36:07 2017 +0200
# Node ID 15c645861efc09cf79cf1c0c6f59cbf8ad1a34c7
# Parent  6675d23da74855a350a8b2dc54166d0b345576da
releasenotes: improve parsing around bullet points

Earlier, on parsing the bullet points from existing release notes the bullet
points after the first one weren't written correctly to the notes file. This
patch makes changes to parsereleasenotesfromfile() function that introduces a new
bullet_points data structure that tracks the bullets and associated subparagraph.
It also makes necessary changes to the tests related to merging of bullets.
Yuya Nishihara - June 19, 2017, 1:26 p.m.
On Tue, 13 Jun 2017 22:38:21 +0200, Rishabh Madan wrote:
> # HG changeset patch
> # User Rishabh Madan <rishabhmadan96@gmail.com>
> # Date 1497386167 -7200
> #      Tue Jun 13 22:36:07 2017 +0200
> # Node ID 15c645861efc09cf79cf1c0c6f59cbf8ad1a34c7
> # Parent  6675d23da74855a350a8b2dc54166d0b345576da
> releasenotes: improve parsing around bullet points

Sorry for the delay. I did some hacks by myself to understand how parsing
works, and a couple of questions came in.

> +    def gatherbullets(offset):
> +        bullet_points = []
> +
> +        for i in range(offset + 1, len(blocks)):
> +            block = blocks[i]
> +
> +            if block['type'] == 'margin':
> +                continue
> +            elif block['type'] == 'section':
> +                break
> +            elif block['type'] == 'bullet':
> +                if block['indent'] != 0:
> +                    raise error.Abort(_('indented bullet lists not supported'))
> +
> +                lines = [[l[1:].strip() for l in block['lines']]]

This function is quite similar to gatherparagraphs(). So maybe the
postprocessing of bullet blocks could be factored out to a function.

> +                if i < len(blocks) - 1:
> +                    i += 1
> +                    block = blocks[i]
> +                    if block['type']=='paragraph':
> +                        lines.append(block['lines'])
> +
> +                    while block['type'] != 'bullet' and i < len(blocks) - 1:
> +                        if block['type'] == 'section':
> +                            break
> +                        i += 1
> +                        block = blocks[i]
> +                        if block['type'] == 'paragraph':
> +                            lines.append(block['lines'])
> +                bullet_points.append(lines)
> +                continue
> +            elif block['type'] != 'paragraph':
> +                raise error.Abort(_('unexpected block type in release notes: '
> +                                    '%s') % block['type'])
> +        return bullet_points
> +
>      def gatherparagraphs(offset):
>          paragraphs = []
>  
> @@ -226,16 +261,18 @@
>                                    title)
>  
>              currentsection = name
> -            paragraphs = gatherparagraphs(i)
> -            if paragraphs:
> -                notes.addnontitleditem(currentsection, paragraphs)
> +            bullet_points = gatherbullets(i)
> +            if bullet_points:
> +                for para in bullet_points:
> +                    notes.addnontitleditem(currentsection, para)

Don't we need to process both paragraphs and bullet points here? I guess we'll
need a function that gathers both paragraphs and bullets in order of
appearance.

> +        if len(notes.nontitledforsection(section)) > 0:
> +            ui.write(_('  bullet point:\n'))
>          for paragraphs in notes.nontitledforsection(section):
> -            ui.write(_('  bullet point:\n'))
>              for para in paragraphs:
>                  ui.write(_('    paragraph: %s\n') % ' '.join(para))

I think 'bullet point:' should be printed per item. Otherwise we can't see if
paragraphs belong to one bullet point or not.
Yuya Nishihara - June 19, 2017, 3:09 p.m.
On Mon, 19 Jun 2017 15:46:06 +0200, Rishabh Madan wrote:
> On Mon, Jun 19, 2017 at 3:26 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> > > @@ -226,16 +261,18 @@
> > >                                    title)
> > >
> > >              currentsection = name
> > > -            paragraphs = gatherparagraphs(i)
> > > -            if paragraphs:
> > > -                notes.addnontitleditem(currentsection, paragraphs)
> > > +            bullet_points = gatherbullets(i)
> > > +            if bullet_points:
> > > +                for para in bullet_points:
> > > +                    notes.addnontitleditem(currentsection, para)
> >
> > Don't we need to process both paragraphs and bullet points here? I guess
> > we'll
> > need a function that gathers both paragraphs and bullets in order of
> > appearance.
> 
> I think I might have created a bit confusion here. Paragraphs simply refer
> to the content under a named title, whereas bullets are non titled content
> (that may also go under 'Other Changes'). In the case here, we are adding
> non-titled sections which basically means just adding the bullet points,
> which is the reason why I just called the gatherbullets function.

Okay, then I think gatherbullets() should raise error if it hits any
paragraphs not belonging to bullets. Or maybe we could add a function that
returns (paragraphs, bullets), and check if paragraphs are empty.

Patch

diff -r 6675d23da748 -r 15c645861efc hgext/releasenotes.py
--- a/hgext/releasenotes.py	Mon Jun 12 17:24:10 2017 +0200
+++ b/hgext/releasenotes.py	Tue Jun 13 22:36:07 2017 +0200
@@ -185,6 +185,41 @@ 
 
     blocks = minirst.parse(text)[0]
 
+    def gatherbullets(offset):
+        bullet_points = []
+
+        for i in range(offset + 1, len(blocks)):
+            block = blocks[i]
+
+            if block['type'] == 'margin':
+                continue
+            elif block['type'] == 'section':
+                break
+            elif block['type'] == 'bullet':
+                if block['indent'] != 0:
+                    raise error.Abort(_('indented bullet lists not supported'))
+
+                lines = [[l[1:].strip() for l in block['lines']]]
+                if i < len(blocks) - 1:
+                    i += 1
+                    block = blocks[i]
+                    if block['type']=='paragraph':
+                        lines.append(block['lines'])
+
+                    while block['type'] != 'bullet' and i < len(blocks) - 1:
+                        if block['type'] == 'section':
+                            break
+                        i += 1
+                        block = blocks[i]
+                        if block['type'] == 'paragraph':
+                            lines.append(block['lines'])
+                bullet_points.append(lines)
+                continue
+            elif block['type'] != 'paragraph':
+                raise error.Abort(_('unexpected block type in release notes: '
+                                    '%s') % block['type'])
+        return bullet_points
+
     def gatherparagraphs(offset):
         paragraphs = []
 
@@ -226,16 +261,18 @@ 
                                   title)
 
             currentsection = name
-            paragraphs = gatherparagraphs(i)
-            if paragraphs:
-                notes.addnontitleditem(currentsection, paragraphs)
+            bullet_points = gatherbullets(i)
+            if bullet_points:
+                for para in bullet_points:
+                    notes.addnontitleditem(currentsection, para)
 
-        elif block['underline'] == '-':  # sub-section
-            paragraphs = gatherparagraphs(i)
-
+        elif block['underline'] == '-':
             if title == BULLET_SECTION:
-                notes.addnontitleditem(currentsection, paragraphs)
+                bullet_points = gatherbullets(i)
+                for para in bullet_points:
+                    notes.addnontitleditem(currentsection, para)
             else:
+                paragraphs = gatherparagraphs(i)
                 notes.addtitleditem(currentsection, title, paragraphs)
         else:
             raise error.Abort(_('unsupported section type for %s') % title)
@@ -423,7 +460,8 @@ 
             for para in paragraphs:
                 ui.write(_('    paragraph: %s\n') % ' '.join(para))
 
+        if len(notes.nontitledforsection(section)) > 0:
+            ui.write(_('  bullet point:\n'))
         for paragraphs in notes.nontitledforsection(section):
-            ui.write(_('  bullet point:\n'))
             for para in paragraphs:
                 ui.write(_('    paragraph: %s\n') % ' '.join(para))
diff -r 6675d23da748 -r 15c645861efc tests/test-releasenotes-merging.t
--- a/tests/test-releasenotes-merging.t	Mon Jun 12 17:24:10 2017 +0200
+++ b/tests/test-releasenotes-merging.t	Tue Jun 13 22:36:07 2017 +0200
@@ -34,8 +34,7 @@ 
   
   * Fix from commit message.
 
-Processing again will no-op
-TODO this is buggy
+Processing again ignores the already added bullet.
 
   $ hg releasenotes -r . $TESTTMP/single-fix-bullet
 
@@ -45,8 +44,6 @@ 
   
   * Fix from release notes.
   
-    Fix from commit message.
-  
   * Fix from commit message.
 
   $ cd ..
@@ -111,7 +108,7 @@ 
 
   $ cd ..
 
-Bullets don't merge properly
+Bullets from rev merge with those from notes file.
 
   $ hg init bullets
   $ cd bullets
@@ -157,7 +154,7 @@ 
   
   * this is fix1.
   
-    this is fix2.
+  * this is fix2.
   
   * this is fix3.