Created attachment 51981 [details] Patch file that kompare cannot parse Version: unspecified (using KDE 4.5.1) OS: Linux Kompare cannot parse the patch attached. Reproducible: Always Steps to Reproduce: 1. Enter command 'kompare kdelibs-kiconloader-cache_returned_pixmaps.patch' Actual Results: Kompare says 'Could not parse diff output' Expected Results: Kompare must show the patch. (Problem can be reproduced in both KDE 4.5 and 4.4. But since I have packages built with debug support only for KDE4.4, output below is from Kompare of KDE 4.4) Here is the command output: gokcen@melmac ~ $ kompare kdelibs-kiconloader-cache_returned_pixmaps.patch kompare(2004)/kdecore (trader): query for mimeType "text/x-patch" , "Kompare/ViewPart" : returning 1 offers kompare(2004)/kdecore (KLibrary): plugins should not have a 'lib' prefix: "libkomparepart.so" kompare(2004)/kompare (libs) Diff2::KompareModelList::KompareModelList: Show me the arguments: 0x8ab9368 , KompareSplitter(0x8ab8f18) , KomparePart(0x8736848) , komparemodellist kompare(2004)/kparts KParts::ReadWritePart::setModified: setModified( false ) kompare(2004)/kdecore (KLibrary): plugins should not have a 'lib' prefix: "libkomparenavtreepart.so" kompare(2004) main: Arg Count = 1 kompare(2004) main: Argument 1 : "kdelibs-kiconloader-cache_returned_pixmaps.patch" kompare(2004) main: Single file. so openDiff/openStdin is only possible... kompare(2004)/kompare (shell) KompareShell::openDiff: Url = "file:///home/gokcen/kdelibs-kiconloader-cache_returned_pixmaps.patch" kompare(2004)/kompare (part) KomparePart::openDiff: Url = "file:///home/gokcen/kdelibs-kiconloader-cache_returned_pixmaps.patch" kompare(2004)/kompare (part) KomparePart::openDiff: Download succeeded kompare(2004)/kompare (libs) Diff2::KompareModelList::openDiff: Stupid :) Url = "/home/gokcen/kdelibs-kiconloader-cache_returned_pixmaps.patch" kompare(2004)/kompare (libs) Diff2::KompareModelList::readFile: Codec = 0x0 kompare(2004)/kompare (nav view) KompareNavTreePart::slotModelsChanged: Models ( 0x0 ) have changed... scanning the models... kompare(2004)/kompare (libs) Diff2::KompareModelList::parseDiffOutput: KompareModelList::parseDiffOutput kompare(2004)/kompare (libs) Diff2::Parser::determineGenerator: Diff is a CVSDiff kompare(2004)/kompare (libs) Diff2::Parser::parse: Cleaned up 0 line(s) of crap from the diff... kompare(2004)/kompare (libs) Diff2::Parser::parse: It is a CVS generated diff... kompare(2004)/kompare (libs) Diff2::KompareModelList::parseDiffOutput: Now i'll be damned, there should be models here !!!
Changing the summary. Now it's better. When I remove index line from the patch, kompare works fine.
Created attachment 51982 [details] Patch file generated via SVN, makes Kompare say "This diff is malformed" This patch file is generated with command: svn di -c 1128360 svn+ssh://svn.kde.org/home/kde/trunk/KDE/kdesdk/ > second.diff and kompare says "This diff is malformed." for that patch. Amazing thing is, the commit that makes kompare give this warning is the patch itself :) The string "This diff is malformed" is added in this commit.
Adding kkofler to CC since those messages may be related to his commits.
I'm already subscribed to kompare-devel, I don't need 2 copies of all the bug messages. ;-)
So the idea behind that patch was that malformed diffs with incorrect line counts were being accepted without any complaints, and some parts of the diffs were just interpreted as comments and not shown. For example, in this testcase: http://ktown.kde.org/~fredrik/folderview.diff where the line counts "@@ -57,8 +56,10 @@" are incorrect (should be "@@ -57,8 +56,9 @@"), it was just eating the second hunk without warning. So I made that change to not just skip random junk, but complain about it. That patch will NOT make Kompare reject a patch entirely though (it'll only complain), and it's also only in 4.5, not 4.4, so this is not the patch which is causing your original bug, it's a separate issue.
The cause of the original bug is ancient code which thinks that any patch from Index: line was generated by CVS. Your diff was actually generated by SVN, which must be using a subtly different format, and so it fails to parse.
(In reply to comment #5) > So the idea behind that patch was that malformed diffs with incorrect line > counts were being accepted without any complaints, and some parts of the diffs > were just interpreted as comments and not shown. For example, in this testcase: > http://ktown.kde.org/~fredrik/folderview.diff > where the line counts "@@ -57,8 +56,10 @@" are incorrect (should be "@@ -57,8 > +56,9 @@"), it was just eating the second hunk without warning. So I made that > change to not just skip random junk, but complain about it. > > That patch will NOT make Kompare reject a patch entirely though (it'll only > complain), and it's also only in 4.5, not 4.4, so this is not the patch which > is causing your original bug, it's a separate issue. So I'll issue a new bug about the wrong "This diff output is malformed" warnings and attach the second.diff patch (second attachment of this report) to the new bug.
Created attachment 51984 [details] Another svn-generated patch file which is shown by Kompare successfully This patch file is also generated via SVN and starts with an "Index: " line. But Kompare can show this file successfully.
So the diff from comment #2 should now work, as per the fix for bug #249976. The first attached diff still fails though and I'm not sure why.
KDE 4.8.3, Kompare 4.1.1, and I still get "Malformed diff" popups, which make me wonder if I really see all the differences that I need to see...
Created attachment 88286 [details] Fixes unified diff parsing for Aegis generated patches This patch fixes unified diff parsing for Aegis generated patches and also makes the patch in the first attachment (51981) of this bugreport work with kompare now. Example patch: aegis.sourceforge.net/cgi-bin/aeget/srecord.1.64.C15/?aepatch+compat=4.25.C681 I have changed the tab character to be optional, but it could have been ignored completely. I just kept it to not lose information. On the other hand I don't know what's the point about being so strict with diff parsing. This looks like having to run after every possible new diff format variant. It seems that there is no fallback for unknown formats, but an abort.
Created attachment 88287 [details] Unified diff file from Aegis (simplified) I have simplified an Aegis generated diff file to a minimal size because for this bug only the lines "Index: ", "--- " and "+++ " are meaningful.
Created attachment 89242 [details] regression test for unified Aegis diff
Any response? Can this simple fix be included?
Kevin, the patch in comment #12 is a one-line change, but I have no idea if it is safe to apply (or if it breaks yet another corner case).
It might be good enough, but as I said in comment #6, I need to look into whether we need a specialized parser for "CVS diffs" at all, which then thinks any diff with an "Index:" line is from CVS, which is just not true. (I don't doubt that this one-character change probably fixes the mentioned test cases, but I think there are probably other assumptions in the "CVS diff" parser which are not true for those other-SCM-generated diffs.) I'll try to have a closer look this week.
Thank you for reporting this bug in KDE software. As it has been a while since this issue was reported, can we please ask you to see if you can reproduce the issue with a recent software version? If you can reproduce the issue, please change the status to "CONFIRMED" when replying. Thank you!
There are still cases when kompare chokes on a diff - mainly when there's non-english errors embedded in it. but that's just a question of what options to pass to the diff command.