Patchwork [v2] releasenotes: improve parsing around bullet points

# 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.

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.

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.