Bug 249976 - kompare complains about malformed diffs
Summary: kompare complains about malformed diffs
Status: RESOLVED INTENTIONAL
Alias: None
Product: kompare
Classification: Unclassified
Component: parser (show other bugs)
Version: 4.1.3
Platform: openSUSE RPMs Linux
: NOR normal with 60 votes (vote)
Target Milestone: ---
Assignee: Kompare developers
URL:
Keywords:
: 252371 339002 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-03 09:14 UTC by Mathias Homann
Modified: 2020-11-12 22:41 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.5.4


Attachments
a diff that gets parsed only partially, or so kompare says. (4.75 KB, text/x-patch)
2010-09-03 09:14 UTC, Mathias Homann
Details
same diff but bzipped (1.74 KB, application/octet-stream)
2010-09-03 16:50 UTC, Mathias Homann
Details
a diff generated by dif -burNE that kompare chokes on (20.61 KB, text/plain)
2020-06-29 08:32 UTC, Mathias Homann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mathias Homann 2010-09-03 09:14:48 UTC
Created attachment 51261 [details]
a diff that gets parsed only partially, or so kompare says.

Version:           4.0.0 (using KDE 4.5.0) 
OS:                Linux

When I open a diff (made with diff -burNE file1 file2), kompare more often than not complains about "malformed diff" and tells me that not all lines are shown in the view.

one such diff is attached.

the same diff parses fine in older versions of kompare.

Reproducible: Sometimes

Steps to Reproduce:
reproducibility sometimes = happens with a lot of diffs but not all of them.
reproduce: open the attached diff in kompare.

Actual Results:  
"the diff is malformed. some lines could not be parsed and will not be displayed in the diff view".

Expected Results:  
I want to see all lines of the diff parsed, or a better explanation as to which ones were malformed.

OS: Linux (i686) release 2.6.34-12-pae
Compiler: gcc
Comment 1 Kevin Kofler 2010-09-03 16:34:37 UTC
Well, FYI, Kompare still displays the exact same thing as before, it just warns you that it might not be complete.

I cannot reproduce this with the diff you attached, maybe it got mangled during processing? Please try reattaching it in a compressed format (.xz / .bz2 / .gz).
Comment 2 Mathias Homann 2010-09-03 16:50:34 UTC
Created attachment 51274 [details]
same diff but bzipped
Comment 3 MartinG 2010-10-17 21:54:32 UTC
*** This bug has been confirmed by popular vote. ***
Comment 4 MartinG 2010-10-17 22:32:46 UTC
I see the same sometimes when I do "svn diff | kompare -o -". If I do "svn diff fileX | kompare -o -" for all differing files separately, I do not get the error.
Comment 5 Christoph Feck 2010-10-19 14:28:12 UTC
*** Bug 252371 has been marked as a duplicate of this bug. ***
Comment 6 Krzysztof Kapuscik 2010-10-20 19:06:47 UTC
I have done some analysis and debugging. The problem is that the svn diff output format is:

Index: somedir/filename.xyz
===================================================================
--- somedir/filename.xyz        (revision 5)
+++ somedir/filename.xyz        (working copy)
@@ -1,3 +1,48 @@
 A = B
-C = D
Index: somedir/filename.abc
===================================================================
--- somedir/filename.abc        (revision 5)
+++ somedir/filename.abc        (working copy)
@@ -1,3 +1,48 @@
 A = B
+C = D

When parsing such file Kompare uses the CVS diff parser with universal format enabled. Unified parses skips first 2 lines (Index and ===) and moves to --- as it is recognized by the regexp. After parsing first hunk the iterator points at next "Index:" but unified parses expects (using function checkHeader()) that the iterator will point at "---" - and the malformed flag is set by checkHeader(). Because the parses actually skips the 'invalid' lines the files gets successfully parsed but the warning popup is still shown.
Comment 7 Kevin Kofler 2010-11-05 23:12:45 UTC
Hmmm, duh, I couldn't reproduce this in comment #1 because I was trying with 4.4.5 whereas I only committed the malformed diff checks to 4.5 (because of the added string).

I can reproduce it now with 4.5.2 (and heck, is it annoying!), I'm working on fixing it.
Comment 8 Kevin Kofler 2010-11-05 23:47:28 UTC
SVN commit 1193415 by kkofler:

Make the malformed diff check more tolerant to prevent choking on SVN-generated diffs or on diffs resulting from the use of diff >>.
It could probably use a better fix than such simple whitelisting, but this should fix the annoyance for now.
Tested on Fedora 13, kdelibs 4.5.2.

BUG: 249976

 M  +3 -1      parserbase.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1193415
Comment 9 Kevin Kofler 2010-11-05 23:49:06 UTC
SVN commit 1193416 by kkofler:

Make the malformed diff check more tolerant to prevent choking on SVN-generated diffs or on diffs resulting from the use of diff >>.
It could probably use a better fix than such simple whitelisting, but this should fix the annoyance for now.
Tested on Fedora 13, kdelibs 4.5.2.

Backport revision 1193415 from trunk.

CCBUG: 249976

 M  +3 -1      parserbase.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1193416
Comment 10 Kevin Kofler 2010-11-06 00:05:03 UTC
Fixed for the upcoming kdesdk 4.5.4 and 4.6.0 releases.
Also fixed in Fedora packaging of 4.5.3 as of kdesdk-4.5.3-3.fc15.
Comment 11 Gökçen Eraslan 2010-11-06 11:34:18 UTC
It's fixed, thank you very much. Can you also look at #252359 ? Kompare still cannot parse some SVN diff outputs :/
Comment 12 Kevin Kofler 2010-11-06 18:03:32 UTC
Hmmm, and another issue with SVN diffs I just thought of now is that sometimes SVN writes lines like "Binary files X and Y differ" in the middle of a diff, which will also trigger the "malformed" complaint. And there I'm not sure how to whitelist them because SVN translates that text. (Plus, ideally, we probably want to display that information somehow, but 1. I don't know how to fit this in Kompare's UI and 2. I really don't know how to parse those messages, since they could be written in ANY language.)
Comment 13 Kevin Kofler 2010-11-06 18:46:31 UTC
PS: Actually, plain diff also reports binary files that differ in that broken way.
Comment 14 Mathias Homann 2012-06-08 12:43:40 UTC
KDE 4.8.3, kompare 4.1.1, bug is back.
Comment 15 Kevin Kofler 2012-06-08 20:55:49 UTC
It's not back. What you're seeing is bug #252359, which is still open.
Comment 16 Mathias Homann 2014-09-11 08:06:19 UTC
Actually in my case the diff I'm trying to parse is *not* generated from svn, I'm passing two folders as argoments to kompare that do not contain version information from .hg or anything other.
There are lots of bitmap files and UTF16 files in the trees, maybe that is what diff chokes on?
Comment 17 Mathias Homann 2014-09-11 08:06:39 UTC
*** Bug 339002 has been marked as a duplicate of this bug. ***
Comment 18 Mathias Homann 2020-06-29 08:31:21 UTC
kompare 4.1.3, bug is still there.
Comment 19 Mathias Homann 2020-06-29 08:32:05 UTC
Created attachment 129754 [details]
a diff generated by dif -burNE that kompare chokes on
Comment 20 Kevin Kofler 2020-06-29 09:18:35 UTC
*** Bug 339002 has been marked as a duplicate of this bug. ***
Comment 21 Kevin Kofler 2020-06-29 09:20:12 UTC
It complains about diff reporting that binary files differ, in a translated message that is not reasonably machine-parsable. I have no way to distinguish this from actually invalid diffs where some parts cannot be displayed. Hence, warning the user about the unparsable lines is by design.
Comment 22 Kevin Kofler 2020-06-29 09:31:31 UTC
And strictly speaking, the warning is actually correct, because the differing binary files cannot be displayed in the diff viewer. (We cannot even show which files differ because we cannot parse the translated messages.)
Comment 23 Mathias Homann 2020-06-29 10:30:50 UTC
(In reply to Kevin Kofler from comment #22)
> parse the translated messages...


NOW we are finally getting somewhere.

I created a script that unsets LANG and then just calls diff with all argumets passed to it, and told kompare to use that script instead of the regular diff command, and voila, no more "malformed diff" errors anymore. Don't even have to exclude image files by pattern anymore.

maybe that script should be part of kompare...

#!/bin/bash
unset LANG
/usr/bin/diff $@

sits in ~/bin on my home folder, and due to my $PATH gets preference before /usr/bin/diff...
Comment 24 Kevin Kofler 2020-06-29 11:01:39 UTC
That would not help with diffs coming from elsewhere (possibly downloaded from the Internet) that you open in Kompare.
Comment 25 Mathias Homann 2020-06-29 12:34:50 UTC
guess i didn't make myself clear enough:

if kompare has problems with non-english output from diff, then kompare should run diff in a way that ensures english output.
Comment 26 Kevin Kofler 2020-06-29 22:46:37 UTC
But Kompare can also be used to view diffs that are already in a .diff file. I cannot control how diff was run in that case.
Comment 27 Mathias Homann 2020-11-12 22:33:26 UTC
(In reply to Kevin Kofler from comment #26)
> But Kompare can also be used to view diffs that are already in a .diff file.
> I cannot control how diff was run in that case.

...in which case we can tell the user about it. But that's no reason not to "do it right" when you *can* control how diff is run.
Comment 28 Mathias Homann 2020-11-12 22:34:35 UTC
by the way, my script to run diff with LANG="" doesn't help anymore for some reason.
Comment 29 Mathias Homann 2020-11-12 22:41:34 UTC
actually that last comment's invalid - the reason for the error *was* because of some binary files...