Skip to content

Commit 9b317c4

Browse files
author
rcartwright
committed
This revision fixes a bug in the updating of document highlights and
improves the implementation of the "jump to line" function in MainFrame.java. My recent string of commits focused on making DrJava more responsive contained a troublesome bug that is proving time-consuming to track down. FindAll result highlights are not updated properly when a document is edited; the highlights do not move with the document. When I backed out the refactoring to minimize the creation of WrappedPositions and the refactoring to defer reconstructing document positions when a kicked out document is remade (waiting until the document becomes active), the highlighting bug went away but when I only backed either of these two refactorings alone, the bug persisted. This revision backs out both of these refactorings. In the long term, I would like to see the reconstruction of wrapped positions deferred until a document becomes active. This optimization should work! I don't understand it can affect the highlight updating process since non-active documents are never edited. On the other hand, the minimization of WrappedPositions may be an inherently flawed idea. If (and I do not yet know the answer) the Swing library internally creates Positions that are critical to updating highlights as a document is edited, then failing to wrap these Positions could sabotage highlight updating. My minimization strategy assumes that no Position internally created by Swing needs to be wrapped. The good news is that this revsion still appears to improve responsiveness despite backing out these two refactorings. The key is a minor change to _jumpToLine in MainFrame.java, which performs the guts of the "go to line" action (CNTL-G). It makes jumping to a line near the end of a large document MUCH faster than it has been in recent builds. The following files were modified or added: M src/edu/rice/cs/drjava/model/cache/DocumentCache.java M src/edu/rice/cs/drjava/model/cache/DocumentCacheTest.java M src/edu/rice/cs/drjava/model/DummyOpenDefDoc.java M src/edu/rice/cs/drjava/model/AbstractGlobalModel.java M src/edu/rice/cs/drjava/model/OpenDefinitionsDocument.java M src/edu/rice/cs/drjava/ui/MainFrame.java M src/edu/rice/cs/drjava/ui/JUnitPanel.java M src/edu/rice/cs/util/FileOps.java M src/edu/rice/cs/util/text/AbstractDocumentInterface.java M src/edu/rice/cs/util/text/SwingDocument.java A src/edu/rice/cs/util/text/SwingDocumentInterface.java A src/edu/rice/cs/util/NullFile.java git-svn-id: file:///tmp/test-svn/trunk@4038 fe72c1cf-3628-48e9-8b72-1c46755d3cff
1 parent fb266d9 commit 9b317c4

File tree

12 files changed

+274
-199
lines changed

12 files changed

+274
-199
lines changed

‎drjava/src/edu/rice/cs/drjava/model/AbstractGlobalModel.java‎

Lines changed: 169 additions & 142 deletions
Large diffs are not rendered by default.

‎drjava/src/edu/rice/cs/drjava/model/DummyOpenDefDoc.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ public Position createPosition(int offs) throws BadLocationException {
228228
return _defDoc.createPosition(offs);
229229
}
230230

231-
public Position createDJPosition(int offs) throws BadLocationException {
232-
return _defDoc.createDJPosition(offs);
231+
public Position createUnwrappedPosition(int offs) throws BadLocationException {
232+
return _defDoc.createUnwrappedPosition(offs);
233233
}
234234

235235
public Element getDefaultRootElement() { return _defDoc.getDefaultRootElement(); }

‎drjava/src/edu/rice/cs/drjava/model/OpenDefinitionsDocument.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,6 @@ public interface OpenDefinitionsDocument extends DJDocument, Finalizable<Definit
341341
/** @return the caret position as set by the view. */
342342
public int getCaretPosition();
343343

344-
/** Creates a Position in the document. Forwards to the embedded SwingDocument. */
345-
public Position createPosition(int offs) throws BadLocationException;
346-
347-
/** Reconstruct the embedded positions in a document (which may been lost because document was kicked out of cache. */
348-
public void makePositions();
344+
/** Creates a WrappedPosition in the document. */
345+
public Position createUnwrappedPosition(int offs) throws BadLocationException;
349346
}

‎drjava/src/edu/rice/cs/drjava/model/cache/DocumentCache.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,12 +173,10 @@ public DocManager(DDReconstructor rec, String fn, boolean isUntitled) {
173173
public DefinitionsDocument getDocument() throws IOException, FileMovedException {
174174
// Utilities.showDebug("getDocument called on " + this + " with _stat = " + _stat);
175175

176-
// The following line is commented out because the double-check can be broken by multi-processor caching or
177-
// compiler optimization. Under Java 5.0, the line is safe provided that _doc is declared volatile.
178-
176+
// The following double-check idiom is safe in Java 1.4 and later JVMs provided that _doc is volatile.
179177
if (_doc != null) return _doc;
180178
synchronized(_cacheLock) { // lock the cache so that this DocManager's state can be updated
181-
if (_doc != null) return _doc; // double-check works for volatile fields in Java 1.4 and later
179+
if (_doc != null) return _doc; // _doc may have changed since test outside of _cacheLock
182180
try { // _doc is not in memory
183181
_doc = _rec.make();
184182
assert _doc != null;
@@ -190,8 +188,6 @@ public DefinitionsDocument getDocument() throws IOException, FileMovedException
190188
}
191189
}
192190

193-
public void makePositions() { _rec.makePositions(); }
194-
195191
/** Checks whether the document is resident (in the cache or modified).
196192
* @return if the document is resident.
197193
*/

‎drjava/src/edu/rice/cs/drjava/model/cache/DocumentCacheTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,6 @@ public DefinitionsDocument make() {
389389
_doc_made++;
390390
return _saved;
391391
}
392-
public void makePositions() { /* do nothing */ }
393392
public void saveDocInfo(DefinitionsDocument doc) { _doc_saved++; }
394393
public void addDocumentListener(javax.swing.event.DocumentListener dl) { /* do nothing */ }
395394
};

‎drjava/src/edu/rice/cs/drjava/ui/JUnitPanel.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ public void testStarted(String name) {
268268
doc.insertString(index, " ", NORMAL_ATTRIBUTES);
269269
index = doc.getLength();
270270
doc.insertString(index, testName + "\n", NORMAL_ATTRIBUTES);
271-
Position pos = doc.createDJPosition(index);
271+
Position pos = doc.createPosition(index);
272272
_runningTestNamePositions.put(fullName, pos);
273273
setCaretPosition(index);
274274
}

‎drjava/src/edu/rice/cs/drjava/ui/MainFrame.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,8 +2110,8 @@ else if (startSel==endSel) {
21102110
}
21112111
DocumentRegion r = _model.getBookmarkManager().getRegionOverlapping(doc, startSel, endSel);
21122112
if (r == null) {
2113-
final Position startPos = doc.createDJPosition(startSel);
2114-
final Position endPos = doc.createDJPosition(endSel);
2113+
final Position startPos = doc.createPosition(startSel);
2114+
final Position endPos = doc.createPosition(endSel);
21152115
SimpleDocumentRegion newR = new SimpleDocumentRegion(doc, doc.getFile(), startPos.getOffset(), endPos.getOffset());
21162116
_model.getBookmarkManager().addRegion(newR);
21172117
}
@@ -4763,12 +4763,12 @@ private File[] getChosenFiles(JFileChooser fc, int choice) throws OperationCance
47634763
private void _selectAll() { _currentDefPane.selectAll(); }
47644764

47654765
/** Jump to the specified line and return the offset.
4766-
* @return offset */
4766+
* @return offset */
47674767
private int _jumpToLine(int lineNum) {
4768-
_currentDefPane.centerViewOnLine(lineNum);
4769-
int pos = _model.getActiveDocument().gotoLine(lineNum);
4770-
_currentDefPane.setCaretPosition(pos);
4771-
return pos;
4768+
int pos = _model.getActiveDocument().gotoLine(lineNum);
4769+
_currentDefPane.setCaretPosition(pos);
4770+
_currentDefPane.centerViewOnOffset(pos);
4771+
return pos;
47724772
}
47734773

47744774
/** Ask the user what line they'd like to jump to, then go there. */
@@ -6447,7 +6447,7 @@ public void uncommentLines() {
64476447
// _currentDefPane.notifyInactive();
64486448
openDoc.setCurrentLocation(start);
64496449
Position startPos;
6450-
try { startPos = openDoc.createPosition(start); }
6450+
try { startPos = openDoc.createUnwrappedPosition(start); }
64516451
catch (BadLocationException e) { throw new UnexpectedException(e); }
64526452

64536453
int startOffset = startPos.getOffset();

‎drjava/src/edu/rice/cs/util/FileOps.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public abstract class FileOps {
5252

5353
private static Log _log = new Log("FileOpsTest.txt", false);
5454

55+
5556
/** Special File object corresponding to a dummy file. Simliar to FileOption.NULL_FILE but exists() returns false. */
5657
public static final File NONEXISTENT_FILE = new File("") {
5758
public String getAbsolutePath() { return ""; }
@@ -60,6 +61,18 @@ public abstract class FileOps {
6061
public boolean exists() { return false; }
6162
};
6263

64+
public static File makeFile(String path) {
65+
File f = new File(path);
66+
try { return f.getCanonicalFile(); }
67+
catch(IOException e) { return f; }
68+
}
69+
70+
public static File makeFile(File parentDir, String child) {
71+
File f = new File(parentDir, child);
72+
try { return f.getCanonicalFile(); }
73+
catch(IOException e) { return f; }
74+
}
75+
6376
/** Determines whether the specified file in within the specified file tree. */
6477
public static boolean inFileTree(File f, File root) {
6578
if (root == null || f == null) return false;

‎drjava/src/edu/rice/cs/util/NullFile.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*BEGIN_COPYRIGHT_BLOCK
2+
*
3+
* This file is part of DrJava. Download the current version of this project from http://www.drjava.org/
4+
* or http://sourceforge.net/projects/drjava/
5+
*
6+
* DrJava Open Source License
7+
*
8+
* Copyright (C) 2001-2006 JavaPLT group at Rice University (javaplt@rice.edu). All rights reserved.
9+
*
10+
* Developed by: Java Programming Languages Team, Rice University, http://www.cs.rice.edu/~javaplt/
11+
*
12+
* Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
13+
* documentation files (the "Software"), to deal with the Software without restriction, including without limitation
14+
* the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and
15+
* to permit persons to whom the Software is furnished to do so, subject to the following conditions:
16+
*
17+
* - Redistributions of source code must retain the above copyright notice, this list of conditions and the
18+
* following disclaimers.
19+
* - Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the
20+
* following disclaimers in the documentation and/or other materials provided with the distribution.
21+
* - Neither the names of DrJava, the JavaPLT, Rice University, nor the names of its contributors may be used to
22+
* endorse or promote products derived from this Software without specific prior written permission.
23+
* - Products derived from this software may not be called "DrJava" nor use the term "DrJava" as part of their
24+
* names without prior written permission from the JavaPLT group. For permission, write to javaplt@rice.edu.
25+
*
26+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO
27+
* THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
28+
* CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
29+
* CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
30+
* WITH THE SOFTWARE.
31+
*
32+
*END_COPYRIGHT_BLOCK*/
33+
34+
package edu.rice.cs.util;
35+
import java.io.File;
36+
import java.io.Serializable;
37+
38+
/** A null file. This class is NOT a singleton because we need multiple, distinct null files for untitled documents.
39+
* The equals method is overridden so that distinct NullFile objects (which all have the same path) are unequal.
40+
*/
41+
public class NullFile extends File implements Serializable {
42+
43+
public NullFile() { super(""); }
44+
public String toString() { return "(Untitled)"; }
45+
46+
/** All distinct objects of type NullFile are unequal. */
47+
public boolean equals(Object o) {
48+
if (o == null || o.getClass() != getClass()) return false;
49+
return o == this;
50+
}
51+
52+
}

‎drjava/src/edu/rice/cs/util/text/AbstractDocumentInterface.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,32 +42,27 @@
4242
public interface AbstractDocumentInterface extends ReadersWritersLocking {
4343

4444
/* Methods from Document interface used in FindReplaceMachine */
45-
46-
/** Returns the length of the document. */
45+
46+
/* Returns the length of the document. */
4747
int getLength();
4848

49-
/** Returns the specified substring of the document. */
49+
/* Returns the specified substring of the document. */
5050
String getText(int offset, int length) throws BadLocationException;
5151

52-
/** Returns the entire text of this document. */
52+
/* Returns the entire text of this document. */
5353
String getText();
5454

55-
/** Inserts given string with specified attributes at the specified offset. */
55+
/* Inserts given string with specified attributes at the specified offset. */
5656
void insertString(int offset, String str, AttributeSet a) throws BadLocationException;
5757

58-
/** Removes the substring of specified length at the specified offset. */
58+
/* Removes the substring of specified length at the specified offset. */
5959
void remove(int offset, int length) throws BadLocationException;
6060

61-
/** Creates a Postion in the document. */
62-
Position createPosition(int offs) throws BadLocationException;
63-
64-
/* New methods that are not in the Document interface. */
65-
6661
/** Appends given string with specified attributes to end of this document. */
6762
void append(String str, AttributeSet set);
6863

69-
/** Creates Positions that persist when a DefinitionsDocument is kicked out of the cache. */
70-
Position createDJPosition(int offs) throws BadLocationException;
64+
/** Creates a "sticky" position within a document */
65+
Position createPosition(int offs) throws BadLocationException;
7166
}
7267

7368

0 commit comments

Comments
 (0)