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

Goffredo Baroncelli kreijack at tiscalinet.it
Mon Jul 23 18:57:40 BST 2007


Hi All,

my last email didn't receive any complaint, so I think that my code goes in 
the right direction.

I addressed most part of the raised concerns:
- the switch --diff was renamed in --show-diff. (John)
- The code handles the (wrong) case where both --show-diff and -m or --file 
were passed. (Jhon)
- The patch include the addendum to the tutorial. (James)
- The encoding of the diff (the most controversial feature) *IS REMOVED*. Now 
the diff content is wrote as 8-bit data on the file passed to the editor.
In any case the diff is not stored in the commit message. So the worst case is 
that the diff in the editor is not clear. 
The down side is the code complication. But I think that is an acceptable 
compromise. (Aaron)
- Added a test case (Aaron)


Below a brief description of the feature

 If you would like to see the diff that will be committed as you edit the
 message you can use the ``--show-diff`` option to ``commit``. This will
 include the diff in the editor when it is opened, below the separator and the
 information about the files that will be committed. This means that you can
 read it as you write the message, but the diff itself wont be seen in the
 commit message when you have finished. If you would like parts to be
 included in the message you can copy and paste them above the separator.


Comments are welcome
Goffredo

--------------
On Thursday 19 July 2007, John Arbash Meinel wrote:
> John Arbash Meinel has voted +0.
> Status is now: Waiting
> Comment:
> I realize I'm coming late to this discussion, but 'commit --show-diff' 
> seems to give a closer meaning than 'commit --diff'.
> 
> +              Option('diff',
> +                     help='Show the diff in the bottom of the 
> editor.'),
> 
> How about:
> 
> Option('diff',
>         help='When no message is supplied, show the diff along with the'
>              ' status summary in the message editor.')
> 

Good point, I didn't considered the combination of the -m and 
the --[show-]diff options

> 
> I would be a lot happier with:
> +        # FIXME: the function show_diff_trees encode the pathname
> +        #        as UTF8. The output of the 
> make_commit_message_template()
> +        #        function is decoded according to the user encoding.
> +        #        So assuming output_encoding = user encoding
> +        #         - if the output_encoding is different from
> +        #        UTF8 the diff body is preserved, but the filename
> +        #        may be not correct
> +        #        - if we set output_encoding=UTF8 and the
> +        #        output_encoding is not UTF8 we have the
> +        #        filepath correct, but the diff body may be not correct
> +
> +        status_tmp.write(stream.getvalue().decode(output_encoding))


Sorry I dont' understand this comment..



> If we could just write the raw stream bytes to the output file, rather 
> than doing any encoding/decoding. I'm guessing it could be done if we 
> changed the layering. But it probably isn't as easy in the current form.

Supposing to share the diff in different environments (unicode+utf8 - 
iso8851-15 / cp850 ), leaving the diff-body as raw-data and the filepath utf8 
encoded may be a good compromise.

But if we use the diff in the same environment where is generated only (as the 
diff used in the commit) and the diff is showed in a editor which is 
_encoding_aware_ I continue to think that the we have to consider the diff 
encoding. 


> This really seems like it is going to puke whenever you have a non-ascii 
> filename modified on Windows. (well, it may not puke since the common 
> encodings are all 8-bit, so the decode() will succeed, it will just give 
> you garbage for the filenames).

Sorry, but I don't understand the last sentence.
If I modify a file in a unix iso8859-15 and if we leave the path UTF8 encoded, 
the editor may show garbage...

> 
> 
> I guess this is better than what we have...

It seems that everybody agree that the diff shall be wrote as raw data.
I am not convinced, but in any case I changed my code....
The down side is that now the edit_commit_message() function and friends
have to handle not only an unicode string, but also a list of strings (unicode 
and not unicode).

Comments are welcome
Goffredo

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) 
<kreijack at inwind_DOT_it>
Key fingerprint = CE3C 7E01 6782 30A3 5B87  87C0 BB86 505C 6B2A CFF9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr-ci-diff.bundle.diff
Type: text/x-diff
Size: 21795 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070723/2bc445a3/attachment-0001.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070723/2bc445a3/attachment-0001.pgp 


More information about the bazaar mailing list