[MERGE] Less bazaar coding style regressions.

[Marius Kruger]

2008/12/11 Robert Collins <robertc at robertcollins.net>

> Robert Collins has voted tweak.
> Status is now: Conditionally approved


BTW. MartinP suggested a different approach on the mailing list,
which I'll pursue later. I think this can go in so long to pave the way.
We can always remove it later if it becomes redundant.

I addressed your concerns, and attached new version.

...

>
> +        bzr_dir = osutils.dirname(osutils.realpath(sys.argv[0]))
>
> better as
> bzr_dir = bzrlib.__path__[0]


I dont really aggree, since we want the workingtree containing the command
being executed,
but I changed it to
bzr_dir = osutils.dirname(bzrlib.__path__[0])



> You have a bug:
> +                    if (changed_content and kind[1] == 'file'
> ... diff
> this will fail if it wasn't a file before.

(it didn't fail, but it didn't pick up style regressions in this remote edge
case).


>
> +                    if (changed_content and kind == ('file', 'file')
>
> will be safe; for the case where kind[0] != 'file', you should just askf or
> the content.
>

fixed.
excellent catch.


>
>
> +    # Special workaround for Python2.3, where difflib fails if
> +    # both sequences are empty.
> +    if not oldlines and not newlines:
> +        return
>

> We don't support python2.3 - no need for this.


:)
this was copied from diff.internal_diff +- line 77
I removed mine now.



> For details, see:
> http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C418c22640812071434nb0ccfc2ma10f851a41bf367d%40mail.gmail.com%3E
> Project: Bazaar
>
>
thanks for looking at this.
regards
marius
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.ubuntu.com/archives/bazaar/attachments/20081213/ad360bdf/attachment.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no_new_trailing_white_space2.patch
Type: text/x-diff
Size: 13700 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20081213/ad360bdf/attachment.bin