From: Frans Klaver <fransklaver@gmail.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Karl Hasselström" <kha@treskal.com>,
"Catalin Marinas" <catalin.marinas@gmail.com>,
"\"Andy Green (林安廸)\"" <andy@warmcat.com>,
git@vger.kernel.org
Subject: Re: [StGit PATCH] Parse commit object header correctly
Date: Wed, 8 Feb 2012 11:43:59 +0100 [thread overview]
Message-ID: <CAH6sp9P=vNjLycgzoWzRbeEsW-kQ5e4HgGYf2jP1+u9rtWV4dg@mail.gmail.com> (raw)
In-Reply-To: <4F3247CA.1020904@alum.mit.edu>
On Wed, Feb 8, 2012 at 11:00 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 02/08/2012 08:33 AM, Junio C Hamano wrote:
>
>> (1) detects end of the hedaer correctly by treating only an empty line as
>> such;
s/hedaer/header/;
>> + line = lines[i].rstrip('\n')
>> + ix = line.find(' ')
>> + if 0 <= ix:
>> + key, value = line[0:ix], line[ix+1:]
>
> The above five lines can be written
>
> for line in lines:
> if ' ' in line:
> key, value = line.rstrip('\n').split(' ', 1)
>
> or (if the lack of a space should be treated more like an exception)
>
> for line in lines:
> try:
> key, value = line.rstrip('\n').split(' ', 1)
> except ValueError:
> continue
This is generally considered more pythonic: "It's easier to ask for
forgiveness than to get permission".
>
>> + 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))
>
> 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
One could also take the recommended python approach for
switch/case-like if/elif/else statements:
updater = { 'tree': lambda cd, value: cd.set_tree(repository.get_tree(value),
'parent': lambda cd, value:
cd.add_parent(repository.get_commit(value)),
'author': lambda cd, value: cd.set_author(Person.parse(value)),
'committer': lambda cd, value:
cd.set_committer(Person.parse(value))
}
for line in lines:
try:
key, value = line.split(' ', 1)
cd = updater[key](cd, value)
except ValueError:
continue
except KeyError:
continue
It documents about the same, but adds checking on double 'case'
statements. The resulting for loop is rather cleaner and the exception
approach becomes even more logical. I rather like the result, but I
guess it's mostly a matter of taste.
Cheers,
Frans
next prev parent reply other threads:[~2012-02-08 10:44 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-07 13:02 STGIT: Deathpatch in linus tree "Andy Green (林安廸)"
2012-02-07 17:37 ` Junio C Hamano
2012-02-08 7:33 ` [StGit PATCH] Parse commit object header correctly Junio C Hamano
2012-02-08 10:00 ` Michael Haggerty
2012-02-08 10:43 ` Frans Klaver [this message]
2012-02-08 16:17 ` Michael Haggerty
2012-02-08 20:04 ` Frans Klaver
2012-02-09 3:58 ` Junio C Hamano
2012-02-15 12:24 ` Catalin Marinas
2012-02-15 18:13 ` Junio C Hamano
2012-02-15 18:40 ` "Andy Green (林安廸)"
2012-02-09 9:38 ` Catalin Marinas
2012-02-09 17:51 ` Jonathan Nieder
2012-02-10 4:27 ` Nguyen Thai Ngoc Duy
2012-02-09 19:04 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAH6sp9P=vNjLycgzoWzRbeEsW-kQ5e4HgGYf2jP1+u9rtWV4dg@mail.gmail.com' \
--to=fransklaver@gmail.com \
--cc=andy@warmcat.com \
--cc=catalin.marinas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kha@treskal.com \
--cc=mhagger@alum.mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.