[MERGE][Take Five] Show the diff in the commit messages

[Aaron Bentley]

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

bb:resubmit

We seem to be finished with code fixes, but there are a lot of style
issues: indenting, long lines.


Goffredo Baroncelli wrote:
> Sorry for the two post, but there was another small clean up to do
> === modified file 'bzrlib/builtins.py'
> --- bzrlib/builtins.py	2007-08-09 15:19:06 +0000
> +++ bzrlib/builtins.py	2007-08-13 17:19:39 +0000
> @@ -2197,6 +2197,9 @@
>                           "the master branch until a normal commit "
>                           "is performed."
>                      ),
> +              Option('show-diff',
> +                     help='When no message is supplied, show the diff along'
> +                     ' with the status summary in the message editor.'),
>               ]
>      aliases = ['ci', 'checkin']
>  
> @@ -2223,12 +2226,13 @@
>          return '\n'.join(properties)
>  
>      def run(self, message=None, file=None, verbose=True, selected_list=None,
> -            unchanged=False, strict=False, local=False, fixes=None):
> +            unchanged=False, strict=False, local=False, fixes=None,
> +            show_diff=False):
>          from bzrlib.commit import (NullCommitReporter, ReportCommitToLog)
>          from bzrlib.errors import (PointlessCommit, ConflictsInTree,
>                  StrictCommitFailed)
> -        from bzrlib.msgeditor import edit_commit_message, \
> -                make_commit_message_template
> +        from bzrlib.msgeditor import edit_commit_message_encoded, \
> +                make_commit_message_template_encoded

^^^ These should be parenthisized, e.g.

        from bzrlib.msgeditor import (edit_commit_message,
                                      make_commit_message_template)

>          # TODO: Need a blackbox test for invoking the external editor; may be
>          # slightly problematic to run this cross-platform.
> @@ -2256,14 +2260,18 @@
>              """Callback to get commit message"""
>              my_message = message
>              if my_message is None and not file:
> -                template = make_commit_message_template(tree, selected_list)
> -                my_message = edit_commit_message(template)
> +                template = make_commit_message_template_encoded(tree, selected_list,
> +                                                        diff=show_diff,
> +                                                        output_encoding=bzrlib.user_encoding)


^^^ indenting here is wrong.  It should line up with "tree", or else be
indented 4 spaces from "template".  Also, you've exceeded 79 chars.

> +            elif (my_message and file ) or \
> +                 (my_message and show_diff ) or \
> +                 (file and show_diff ):

^^^ Again, you can use parentheses to avoid \

I agree that it would be nice to just ignore show_diff if -m or --file
is speficifed.

>  
>      extra_trees
>          If set, more Trees to use for looking up file ids
> +
> +    path_encoding
> +        If set, the path will be encoded as specified

^^^ It should say that encoding is always done, and is utf-8 by default.
 This makes it sound like no encoding will be done if the path_encoding
isn't specified.

> @@ -436,7 +441,9 @@
>      has_changes = 0
>      for path, file_id, kind in delta.removed:
>          has_changes = 1
> -        print >>to_file, "=== removed %s '%s'" % (kind, path.encode('utf8'))
> +        print >>to_file, "=== removed %s '%s'" % (kind,
> +                                                path.encode(path_encoding,
> +                                                            "replace"))

^^^ fix indenting

> -        print >>to_file, "=== added %s '%s'" % (kind, path.encode('utf8'))
> +        print >>to_file, "=== added %s '%s'" % (kind, path.encode(path_encoding,
> +                                                                "replace"))

^^^ fix indenting

> @@ -456,8 +464,8 @@
>          has_changes = 1
>          prop_str = get_prop_change(meta_modified)
>          print >>to_file, "=== renamed %s '%s' => %r%s" % (
> -                    kind, old_path.encode('utf8'),
> -                    new_path.encode('utf8'), prop_str)
> +                    kind, old_path.encode(path_encoding, "replace"),
> +                    new_path.encode(path_encoding, "replace"), prop_str)

^^^ old indenting is wrong.  Please fix, since you're modifying.

> @@ -470,8 +478,10 @@
>      for path, file_id, kind, text_modified, meta_modified in delta.modified:
>          has_changes = 1
>          prop_str = get_prop_change(meta_modified)
> -        print >>to_file, "=== modified %s '%s'%s" % (kind, path.encode('utf8'),
> -                                                     prop_str)
> +        print >>to_file, "=== modified %s '%s'%s" % (kind,
> +                                                   path.encode(path_encoding,
> +                                                               "replace"),
> +                                                   prop_str)

^^^ Please fix indenting.

> +def make_commit_message_template_encoded(working_tree, specific_files,
> +                                 diff=None,
> +                                 output_encoding=None):
> +    """Prepare a template file for a commit into a branch.
> +
> +    Returns an encoded string.
> +    """
> +    # TODO: Should probably be given the WorkingTree not the branch

^^^ please remove TODO

> === modified file 'bzrlib/tests/test_msgeditor.py'
> --- bzrlib/tests/test_msgeditor.py	2007-05-01 09:09:18 +0000
> +++ bzrlib/tests/test_msgeditor.py	2007-07-26 22:38:49 +0000
> @@ -52,6 +52,35 @@
>    hell\u00d8
>  """)
>  
> +    def test_commit_template_encoded(self):
> +        """Test building a commit message template"""
> +        working_tree = self.make_uncommitted_tree()
> +        template = bzrlib.msgeditor.make_commit_message_template_encoded(working_tree,
> +                                                     None,
> +                                                     output_encoding='utf8')
> +        self.assertEqualDiff(template,

^^^ fix indenting, line exceeds 79 chars


> +    def test_commit_template_and_diff(self):
> +        """Test building a commit message template"""
> +        working_tree = self.make_uncommitted_tree()
> +        template = bzrlib.msgeditor.make_commit_message_template_encoded(working_tree,
> +                                                     None, diff=True,
> +                                                     output_encoding='utf8')

^^^ fix indenting

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

iD8DBQFGxHgt0F+nu1YWqI0RAvxfAJ9u0d84LL3qatXWpvPRUIFfFAiuegCfTFOR
foei9mX996cVvQeMM8YEF3M=
=ldTg
-----END PGP SIGNATURE-----