[MERGE] (0.17) reworked LogFormatter API

[Kent Gibson]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



John Arbash Meinel wrote:
> Kent Gibson wrote:
>> Hi John,
>
>> Can you point out where in PEP-8 is specifies that there must be
>> space in the cases you highlight?  The only section I can find
>> relates to not adding extraneous space.
>
>> Not that I have a problem with it if it is the bzr way, just I
>> don't see it in PEP-8.
>
> Read the Whitespace in Expressions and Statements section
> thoroughly.
>
> It starts with: Yes: spam(ham[1], {eggs: 2}) No:  spam( ham[ 1 ], {
>  eggs: 2 } )
>
> and Yes: if x == 4: print x, y; x, y = y, x No:  if x == 4 : print
> x , y ; x , y = y , x
>
> and Yes:
>
> x = 1 y = 2 long_variable = 3
>
> No:
>
> x             = 1 y             = 2 long_variable = 3
>
> and eventually gets to: Yes:
>
> def complex(real, imag=0.0): return magic(r=real, i=imag)
>
> No:
>
> def complex(real, imag = 0.0): return magic(r = real, i = imag)
>
Yeah, that is showing extraneous space in the No, around the "=".
>
>>> -    if show_merge is not None and show_merge_revno is None: +
>>> legacy_lf = not getattr(lf, 'log_revision', None) I would also
>>> probably do this as: legacy_lf = getattr(...) is not None
>> I hope you mean:
>
>> legacy_lf = getattr() is None
>
>> And I prefer my way anyway - I think it reads better.
>
> Well, you really are checking against None, not just a parameter
> which evaluates to false.
Actually I should be checking that the result is callable, but that
will come out in the wash.

>
> What we really want is 'hasattr()' but unfortunately hasattr()
> supresses too many exceptions.
>
Good to know.  I normally would've used hasattr, but I followed the
existing code that used getattr, and just negated it to get what I was
after.

> Normally I actually use the result directly. So I do: log_revision
> = getattr(lf, 'log_revision', None) if log_revision is None: #
> legacy ...
>
> else: log_revision(foo)
>
Yup, I prefer that too.
>
>
> I personally don't find "not getattr()" to be especially readable.
> 'not hasattr()' is more obvious, but we avoid that for other
> reasons.
>
> Anyway, I've sent this to PQM, though you are welcome to submit a
> patch for review if you don't like how I've done some of it.
>
I don't care either way - I just hope you got the logic around the
right way.

Cheers,
Kent.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGVYiogoxTFTi1P8QRAuAvAJ9JZIIhzDZ69BoNmc+IcEGIayYAbQCg3eV9
6R658e/d3f0hR1Cp+3LowHQ=
=iFc0
-----END PGP SIGNATURE-----