This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 57515 - Diff - built-in: shows incorrect changes.
Summary: Diff - built-in: shows incorrect changes.
Status: CLOSED FIXED
Alias: None
Product: utilities
Classification: Unclassified
Component: Diff (show other bugs)
Version: 4.x
Hardware: All Windows XP
: P2 blocker (vote)
Assignee: Martin Entlicher
URL:
Keywords: REGRESSION
Depends on: 42638
Blocks:
  Show dependency tree
 
Reported: 2005-04-06 08:26 UTC by Peter Pis
Modified: 2006-03-24 12:55 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
diff built-in engine (38.64 KB, image/png)
2005-04-06 08:29 UTC, Peter Pis
Details
file, patch and file after patch (752 bytes, application/x-compressed)
2005-04-12 10:09 UTC, Peter Pis
Details
The textual patch that fixes this issue. (7.83 KB, patch)
2005-04-13 15:08 UTC, Martin Entlicher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Pis 2005-04-06 08:26:40 UTC
Product Version       = NetBeans IDE 4.1 RC1 (Build 200504051930)
Operating System      = Windows XP version 5.1 running on x86
Java; VM; Vendor      = 1.5.0_03; Java HotSpot(TM) Client VM 1.5.0_03-b05; Sun
Microsystems Inc.
Java Home             = d:\Java\jdk1.5.0_03\jre

Steps:
1. Create patch("CVS | Patch"), save to the file.
2. Then apply it to the file and view changes. 
3. Switch to "Built-in engine".

It shows wrong results.
Comment 1 Peter Pis 2005-04-06 08:29:14 UTC
Created attachment 21407 [details]
diff built-in engine
Comment 3 Martin Entlicher 2005-04-06 11:01:28 UTC
Well, but I'm afraid that this is not related to the problem in javacvs library.
The built-in CVS client is not used in this issue, only the "built-in diff engine".

I could not reproduce this so far, Peter, please attach the file *before* the
patch was applied, *after* the patch was applied and the patch file. Thanks.
Comment 4 Martin Entlicher 2005-04-06 11:02:34 UTC
And also, attach them as *binary* files. This is very important so that the
original end of lines will be retained.
Comment 5 Martin Entlicher 2005-04-06 14:52:41 UTC
I've actually reproduced it. But one must use a file, which has Window's line
endings (\r\n). Therefore this can not be reproduced with files created from
NetBeans templates.

It looks like a bug in patch action where the GUI diff is created. Because the
patch is applied correctly (with the correct newlines).
Comment 6 Martin Entlicher 2005-04-06 17:27:11 UTC
The problem is, that when comparing the two files, one is read directly from the
disk, but the other is read through the EditorSupport (the hack introduced
because of proper localization).

This mismatch is caused by a bug in DefaultDataObject - it does not return
EditCookie.

The fix is trivial:

Index: DefaultDataObject.java
===================================================================
RCS file: /cvs/openide/loaders/src/org/openide/loaders/DefaultDataObject.java,v
retrieving revision 1.7
diff -u -r1.7 DefaultDataObject.java
--- DefaultDataObject.java	7 Feb 2005 13:53:26 -0000	1.7
+++ DefaultDataObject.java	6 Apr 2005 16:26:23 -0000
@@ -166,6 +166,8 @@
         }
             
         if (
+            c.isAssignableFrom(org.openide.cookies.EditCookie.class)
+            ||
             c.isAssignableFrom(org.openide.cookies.EditorCookie.Observable.class)
             ||
             c.isAssignableFrom (org.openide.cookies.PrintCookie.class)
Comment 7 Jaroslav Tulach 2005-04-07 18:41:27 UTC
Ok, DefaultDataObject does not provide EditCookie, but why it should? I cannot 
imagine what you do that can get broken by not providing it. 
Comment 8 Martin Entlicher 2005-04-07 19:18:47 UTC
DefaultDataObject normally provides EditCookie, when all cookies are
initialized. But initially, when there is a request for EditCookie, it returns
null, because there is missing the provided condition.
Comment 9 Jaroslav Tulach 2005-04-07 19:25:15 UTC
Checking in loaders/src/org/openide/loaders/DefaultDataObject.java;
/cvs/openide/loaders/src/org/openide/loaders/DefaultDataObject.java,v  <-- 
DefaultDataObject.java
new revision: 1.8; previous revision: 1.7
done
Checking in test/unit/src/org/openide/loaders/DefautDataObjectHasOpenTest.java;
/cvs/openide/test/unit/src/org/openide/loaders/DefautDataObjectHasOpenTest.java,v
 <--  DefautDataObjectHasOpenTest.java
new revision: 1.11; previous revision: 1.10
Comment 10 Jaroslav Tulach 2005-04-08 16:11:58 UTC
"#57515: Backport of EditCookie for DefaultDataObject" 
openide/test/unit/src/org/openide/loaders/DefautDataObjectHasOpenTest.java 
openide/loaders/src/org/openide/loaders/DefaultDataObject.java 
Checking in 
openide/test/unit/src/org/openide/loaders/DefautDataObjectHasOpenTest.java; 
/cvs/openide/test/unit/src/org/openide/loaders/DefautDataObjectHasOpenTest.java,v  
<--  DefautDataObjectHasOpenTest.java 
new revision: 1.10.14.1; previous revision: 1.10 
done 
Checking in openide/loaders/src/org/openide/loaders/DefaultDataObject.java; 
/cvs/openide/loaders/src/org/openide/loaders/DefaultDataObject.java,v  <--  
DefaultDataObject.java 
new revision: 1.7.12.1; previous revision: 1.7 
done 
 
Comment 11 Marian Mirilovic 2005-04-12 07:00:27 UTC
Peter,
please verify in NB4.1, thanks in advance.
Comment 12 Peter Pis 2005-04-12 08:53:44 UTC
I can still reproduce this in NetBeans IDE Dev (Build 200504111930) and in
NetBeans IDE Dev (Build 200504111800).
Comment 13 Marian Mirilovic 2005-04-12 08:56:00 UTC
Just small correction of Peter's comment :
I can still reproduce this in NetBeans IDE 4.1 (Build 200504111930) and in
NetBeans IDE 4.2/Dev (Build 200504111800).
Comment 14 Peter Pis 2005-04-12 10:09:46 UTC
Created attachment 21567 [details]
file, patch and file after patch
Comment 15 Martin Entlicher 2005-04-12 15:45:54 UTC
The patch is applied correctly. All files have Windows newlines. Thus the
problem seems to remain in the diffing. Will try to reproduce...
Comment 16 Martin Entlicher 2005-04-13 11:42:13 UTC
Reproduced. The problem is not much solvable in general, until issue #42638 is
solved.

This is caused by the fact that one file (.orig) is loaded through Document
(which converts all line-endings to '\n') and the other (.java) is loaded
directly from the file via a Reader with the proper encoding. This is because we
can obtain the encoding for Java files, but not for other file types.

So, the ApplyPatch action can be made smart to detect this case. It must assure
that the reader is retrieved through the same conversion mechanism for both
original and patched file.
DiffAction must take care that when comparing Java files with other files, they
must both be loaded through a Document.
Comment 17 Martin Entlicher 2005-04-13 15:06:52 UTC
Fixed in trunk.
 When two files are compared, either both of them or none use the encoding
 hack through EditorKit, which assures that we get consistent line endings.
 Also the patch supplies the file which should determine the encoding logic.

/cvs/diff/src/org/netbeans/modules/diff/DiffAction.java,v  <--  DiffAction.java
new revision: 1.27; previous revision: 1.26

/cvs/diff/src/org/netbeans/modules/diff/EncodedReaderFactory.java,v  <-- 
EncodedReaderFactory.java
new revision: 1.5; previous revision: 1.4

/cvs/diff/src/org/netbeans/modules/diff/PatchAction.java,v  <--  PatchAction.java
new revision: 1.25; previous revision: 1.24
Comment 18 Martin Entlicher 2005-04-13 15:07:40 UTC
This belongs to diff - moving to diff module.
Comment 19 Martin Entlicher 2005-04-13 15:08:22 UTC
Created attachment 21614 [details]
The textual patch that fixes this issue.
Comment 20 Peter Pis 2005-04-14 12:33:56 UTC
Verified in NetBeans IDE Dev (Build 200504131800).
Comment 21 _ pkuzel 2005-04-14 15:29:36 UTC
+        } finally {
+            try {
+                if (r1 != null) r1.close();
+                if (r2 != null) r2.close();
+            } catch (IOException ioex) {}

can leave the second reader opened.

REVIEWED
Comment 22 Martin Entlicher 2005-04-14 16:05:18 UTC
Good point, thanks.
Fixed the try/catch in trunk:
/cvs/diff/src/org/netbeans/modules/diff/DiffAction.java,v  <--  DiffAction.java
new revision: 1.28; previous revision: 1.27
Comment 23 Martin Entlicher 2005-04-14 16:10:58 UTC
Thanks for the review, the fix is merged into release41 branch:

/cvs/diff/src/org/netbeans/modules/diff/PatchAction.java,v  <--  PatchAction.java
new revision: 1.23.2.2; previous revision: 1.23.2.1

/cvs/diff/src/org/netbeans/modules/diff/EncodedReaderFactory.java,v  <-- 
EncodedReaderFactory.java
new revision: 1.3.6.2; previous revision: 1.3.6.1

/cvs/diff/src/org/netbeans/modules/diff/DiffAction.java,v  <--  DiffAction.java
new revision: 1.25.2.2; previous revision: 1.25.2.1