From: Catalin Marinas <catalin.marinas@gmail.com>
Date: Wed, 15 Feb 2012 12:24:34 GMT
Subject: Re: [StGit PATCH] Parse commit object header correctly
Message-ID: <CAHkRjk451=_XaQuUXmxAvB3sRRz6-J+c7A2ZrfLwfGz=z05Lag@mail.gmail.com>
In-Reply-To: <4F3247CA.1020904@alum.mit.edu>

On 8 February 2012 10:00, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 02/08/2012 08:33 AM, Junio C Hamano wrote:
>> To allow parsing the header produced by versions of Git newer than the
>> code written to parse it, all commit parsers are expected to skip unknown
>> header lines, so that newer types of header lines can be added safely.
>> The only three things that are promised are:
>>
>>  (1) the header ends with an empty line (just an LF, not "a blank line"),
>>  (2) unknown lines can be skipped, and
>>  (3) a header "field" begins with the field name, followed by a single SP
>>      followed by the value.
>>
>> The parser used by StGit, introduced by commit cbe4567 (New StGit core
>> infrastructure: repository operations, 2007-12-19), was accidentally a bit
>> too loose to lose information, and a bit too strict to raise exception
>> when dealing with a line it does not understand.
...
> All in all, I would recommend something like (untested):
>
>        @return: A new L{CommitData} object
>        @rtype: L{CommitData}"""
>        cd = cls(parents = [])
>        lines = []
>        raw_lines = s.split('\n')
>        # Collapse multi-line header lines
>        for i, line in enumerate(raw_lines):
>            if not line:
>                cd.set_message('\n'.join(raw_lines[i+1:]))
>                break
>            if line.startswith(' '):
>                # continuation line
>                lines[-1] += '\n' + line[1:]
>            else:
>                lines.append(line)
>
>        for line in lines:
>            if ' ' in line:
>                key, value = line.split(' ', 1)
>                if key == 'tree':
>                    cd = cd.set_tree(repository.get_tree(value))
>                elif key == 'parent':
>                    cd = cd.add_parent(repository.get_commit(value))
>                elif key == 'author':
>                    cd = cd.set_author(Person.parse(value))
>                elif key == 'committer':
>                    cd = cd.set_committer(Person.parse(value))
>        return cd

Thank you all for comments and patches. I used a combination of
Junio's patch with the comments from Michael and a fix from me. I'll
publish it to the 'master' branch shortly and release a 0.16.1
hopefully this week.

-- 
Catalin