Qeole / colorediffs

Thunderbird extension to colorize code diffs in messages.
Mozilla Public License 2.0
24 stars 5 forks source link

Unified diffs section headings are reformatted as context #87

Closed rohieb closed 6 years ago

rohieb commented 6 years ago

Consider an email containing the following diff:

--- a/rules/systemd.in
+++ b/rules/systemd.in
@@ -136,6 +136,7 @@ menu "install options               "

 config SYSTEMD_UDEV_DRIVERS_RULES
    bool
+   default y
    prompt "install udev driver rules"
    help
      This will install the generic udev drivers rules files from the udev package

This diff contains a section heading menu "install options " in the hunk info. When I view this diff in version 0.7 with the "unified" view, the header is reformatted so that it appears as the first context line:

=======================================
diff --git a/rules/systemd.in b/rules/systemd.in
index 95cc3c792ffe..266c83fc80f0 100644
--- a/rules/systemd.in
+++ b/rules/systemd.in
@@ -136,7 +136,8 @@
 menu "install options               " 

 config SYSTEMD_UDEV_DRIVERS_RULES 
    bool 
+   default y 
    prompt "install udev driver rules" 
    help 
      This will install the generic udev drivers rules files from the udev package 

The line numbers in the hunk info are recalculated accordingly, so in this case the hunk applies cleanly, but this is not always the case: Header lines can be anything the user wants (mostly the C function which the change was in), but are only interesting for the reader, and not actually interpreted when applying the patch.

I think a fix could be as simple as changing the regex in https://github.com/Qeole/colorediffs/blob/8287043/chrome/content/parsers/unified-parser.js#L374 to include the optional header.

Qeole commented 6 years ago

Hi, and thanks for the report!

I've never tried to apply directly from a colored view, so this has never been an issue for me (and I had never realised the info line was renumbered as it if was part of the diff).

Thanks for pointing to the regex too, although just changing the regex might end up coloring the info hunk as well, which might not be suitable. But I'll definitely have a look into that and find a way to fix it.

Qeole commented 6 years ago

Hi @rohieb,

Could you please try to install from the branch git_infoline of this repo and tell me if it fixes your issue? Display looks fine to me, but I haven't tried to apply the colored patches.

(Uninstall add-on, clone the branch, cd colorediffs, make, and install add-on from generated xpi file)

rohieb commented 6 years ago

Oh. It seems my mail did not reach GitHub, so for reference, that branch fixes my issue :)

Qeole commented 6 years ago

Great! Thanks a lot for testing. I'll try to finish the fixes for compatibility with Thunderbird 59 and then I'll push a new version to AMO.