[PATCH v2] releasenotes: improve parsing around bullet points

[Rishabh Madan]

On Mon, Jun 19, 2017 at 3:26 PM, Yuya Nishihara <yuya at tcha.org> wrote:

> On Tue, 13 Jun 2017 22:38:21 +0200, Rishabh Madan wrote:
> > # HG changeset patch
> > # User Rishabh Madan <rishabhmadan96 at 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.
>

Yes I too think there is a lot of code duplication here. I think I can
figure out something and combine the gatherparagraphs and gatherbullets
function into a single 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.


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.

>
> > +        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.
>
I agree. I'll make necessary changes for this part.
ᐧ
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170619/1016b672/attachment.html>