On Wed, Feb 8, 2012 at 11:00 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> 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
��.n��������+%������w��{.n��������n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�