Skip to content

Commit 577d0df

Browse files
committed
\This revision cleans up the code in FindReplaceMachine.java which was
ugly. Important parameters were sometimes bound as field and sometimes as method arguments. I converted the code to exclusively use parameters bound as fields and it eliminated some undiagnosed bugs. The following files were modified, added, or deleted: modified: build.xml modified: lib/buildlib/junit.jar modified: lib/plt.jar modified: src/edu/rice/cs/drjava/DrJava.java modified: src/edu/rice/cs/drjava/config/ResourceBundleConfiguration.java modified: src/edu/rice/cs/drjava/model/AbstractGlobalModel.java modified: src/edu/rice/cs/drjava/model/FindReplaceMachine.java modified: src/edu/rice/cs/drjava/model/GlobalModelCompileTest.java modified: src/edu/rice/cs/drjava/model/JDKDescriptor.java modified: src/edu/rice/cs/drjava/model/JarJDKToolsLibrary.java modified: src/edu/rice/cs/drjava/model/compiler/CompilerErrorModel.java modified: src/edu/rice/cs/drjava/model/compiler/DefaultCompilerModel.java modified: src/edu/rice/cs/drjava/model/compiler/JavacCompiler.java modified: src/edu/rice/cs/drjava/model/debug/jpda/JPDADebugger.java modified: src/edu/rice/cs/drjava/model/definitions/indent/ActionBracePlusTest.java modified: src/edu/rice/cs/drjava/model/definitions/indent/ActionStartPrevLinePlusMultilinePreserveTest.java modified: src/edu/rice/cs/drjava/model/junit/ConcJUnitUtils.java modified: src/edu/rice/cs/drjava/ui/DetachedFrame.java modified: src/edu/rice/cs/drjava/ui/ErrorPanel.java modified: src/edu/rice/cs/drjava/ui/MainFrame.java modified: src/edu/rice/cs/drjava/ui/RegionsTreePanel.java modified: src/edu/rice/cs/drjava/ui/SmartSourceFilter.java modified: src/edu/rice/cs/drjava/ui/config/ConfigFrame.java modified: src/edu/rice/cs/util/Log.java modified: src/edu/rice/cs/util/MD5ChecksumProperties.java modified: ../plt/build.xml modified: ../plt/src/edu/rice/cs/plt/reflect/PathClassLoader.java modified: ../plt/src/edu/rice/cs/plt/reflect/ShadowingClassLoader.java
1 parent 666dbbb commit 577d0df

28 files changed

+122
-137
lines changed

drjava/build.xml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@
577577
<available property="junit-jar-to-include-exists" file="${junit-jar-to-include}" />
578578
<fail message="Can't find ${junit-jar-to-include}"
579579
unless="junit-jar-to-include-exists" />
580-
<jar jarfile="${ant.project.name}.jar">
580+
<jar jarfile="${version-tag}.jar">
581581
<manifest>
582582
<attribute name="Main-Class" value="${main-class}" />
583583
<attribute name="Built-By" value="${user.name}" />
@@ -637,10 +637,10 @@
637637
</classpath>
638638

639639
<link href="http://docs.oracle.com/javase/8/docs/api/"/>
640-
<link href="http://junit.org/junit/javadoc/3.8.1" />
641-
<link href="http://drjava.org/javadoc/plt" />
642-
<link href="http://drjava.org/javadoc/javalanglevels" />
643-
<link href="http://drjava.org/javadoc/dynamicjava" />
640+
<link href="http://junit.org/junit4/javadoc/4.12/index.html?org/junit/"/>
641+
<link href="http://drjava.org/javadoc/plt/" />
642+
<link href="http://drjava.org/javadoc/javalanglevels/" />
643+
<link href="http://drjava.org/javadoc/dynamicjava/" />
644644
<!-- Additional external library APIs may be listed here -->
645645
</javadoc>
646646
</target>

drjava/lib/buildlib/junit.jar

60.3 KB
Binary file not shown.

drjava/lib/plt.jar

-1 Bytes
Binary file not shown.

drjava/src/edu/rice/cs/drjava/DrJava.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ protected static void _saveConfig() {
554554
// String selectedVersion = _getToolsJarVersion(selectedFile);
555555
//
556556
// final String[] text;
557-
// if (selectedVersion==null) {
557+
// if (selectedVersion == null) {
558558
// text = new String[] {
559559
// "The file you chose did not appear to be the correct 'tools.jar'",
560560
// "that is compatible with the version of Java that is used to",

drjava/src/edu/rice/cs/drjava/config/ResourceBundleConfiguration.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public ResourceBundleConfiguration(String resourceBundleName, FileConfiguration
6565
_shadowed = shadowed;
6666
map = new OptionMap() {
6767
public <T> T getOption(OptionParser<T> o) {
68-
if (o==null) return _shadowed.getOptionMap().getOption(o);
68+
if (o == null) return _shadowed.getOptionMap().getOption(o);
6969
try {
7070
String str = _bundle.getString(o.getName());
7171
return o.parse(str); // defined in resource bundle
@@ -89,7 +89,7 @@ public <T> T setOption(Option<T> o, T val) {
8989
}
9090

9191
public <T> String getString(OptionParser<T> o) {
92-
if (o==null) return _shadowed.getOptionMap().getString(o);
92+
if (o == null) return _shadowed.getOptionMap().getString(o);
9393
try {
9494
String str = _bundle.getString(o.getName());
9595
return str; // defined in resource bundle
@@ -101,7 +101,7 @@ public <T> String getString(OptionParser<T> o) {
101101
}
102102

103103
public <T> void setString(OptionParser<T> o, String s) {
104-
if (o==null) _shadowed.getOptionMap().setString(o, s);
104+
if (o == null) _shadowed.getOptionMap().setString(o, s);
105105
try {
106106
_bundle.getString(o.getName());
107107
return; // defined in resource bundle, can't be set
@@ -113,7 +113,7 @@ public <T> void setString(OptionParser<T> o, String s) {
113113
}
114114

115115
public <T> T removeOption(OptionParser<T> o) {
116-
if (o==null) return _shadowed.getOptionMap().removeOption(o);
116+
if (o == null) return _shadowed.getOptionMap().removeOption(o);
117117
try {
118118
_bundle.getString(o.getName());
119119
return null; // defined in resource bundle, can't be removed
@@ -143,7 +143,7 @@ public Iterable<OptionParser<?>> keys() {
143143
* @param value New value for the option
144144
*/
145145
public <T> T setSetting(final Option<T> op, final T value) {
146-
if (op==null) return _shadowed.setSetting(op, value);
146+
if (op == null) return _shadowed.setSetting(op, value);
147147
try {
148148
_bundle.getString(op.getName());
149149
return null; // defined in resource bundle, can't be set
@@ -156,7 +156,7 @@ public <T> T setSetting(final Option<T> op, final T value) {
156156

157157
/** Gets the current value of the given Option. */
158158
public <T> T getSetting(Option<T> op) {
159-
if (op==null) return _shadowed.getSetting(op);
159+
if (op == null) return _shadowed.getSetting(op);
160160
try {
161161
String str = _bundle.getString(op.getName());
162162
return op.parse(str); // defined in resource bundle
@@ -169,7 +169,7 @@ public <T> T getSetting(Option<T> op) {
169169

170170
/** Return true if the option is editable. If it was defined in the resource bundle, it is not editable. */
171171
public <T> boolean isEditable(Option<T> op) {
172-
if (op==null) return _shadowed.isEditable(op);
172+
if (op == null) return _shadowed.isEditable(op);
173173
try {
174174
_bundle.getString(op.getName());
175175
return false; // defined, not editable

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1469,7 +1469,7 @@ public void openFolder(File dir, boolean rec, String ext)
14691469
/** @return the file extension for the "Open Folder..." command for the currently selected compiler. */
14701470
public String getOpenAllFilesInFolderExtension() {
14711471
CompilerModel cm = getCompilerModel();
1472-
if (cm==null) {
1472+
if (cm == null) {
14731473
return OptionConstants.LANGUAGE_LEVEL_EXTENSIONS[DrJava.getConfig().getSetting(LANGUAGE_LEVEL)];
14741474
}
14751475
else {

drjava/src/edu/rice/cs/drjava/model/FindReplaceMachine.java

Lines changed: 37 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,23 @@ public class FindReplaceMachine {
5858

5959
/* Visible machine state; manipulated directly or indirectly by FindReplacePanel. */
6060
private volatile OpenDefinitionsDocument _doc; // Current search document
61-
private volatile OpenDefinitionsDocument _firstDoc; // First document where searching started (when searching all documents)
61+
private volatile OpenDefinitionsDocument _firstDoc; // First document where searching started (in all docs modes)
6262
private volatile int _current; // Position of the cursor in _doc when machine is stopped
6363
private volatile MovingDocumentRegion _selectionRegion; // selected text region
6464
private volatile String _findWord; // Word to find. */
6565
private volatile String _replaceWord; // Word to replace _findword.
6666
private volatile boolean _matchCase;
6767
private volatile boolean _matchWholeWord;
68-
private volatile boolean _searchAllDocuments; // Whether to search all documents (or just the current document)
68+
private volatile boolean _searchAllDocuments; // Whether to search all documents (or just current document)
6969
private volatile boolean _searchSelectionOnly; // Whether to search only the selection
7070
private volatile boolean _isForward; // Whether search direction is forward (false means backward)
7171
private volatile boolean _ignoreCommentsAndStrings; // Whether to ignore matches in comments and strings
7272
private volatile boolean _ignoreTestCases; // Whether to ignore documents that end in *Test.java
73-
private volatile String _lastFindWord; // Last word found; set to null by FindReplacePanel if caret is updated
74-
private volatile boolean _skipText; // Whether to skip over the current match if direction is reversed
73+
private volatile String _lastFindWord; // Last word found; set to null by FindReplacePanel if caret updated
74+
private volatile boolean _skipText; // Whether to skip over the current match if direction reversed
7575
private volatile DocumentIterator _docIterator; // An iterator of open documents; _doc is current
7676
private volatile SingleDisplayModel _model;
77-
private volatile Component _frame;
77+
private volatile Component _frame;
7878

7979
/** Standard Constructor.
8080
* Creates new machine to perform find/replace operations on a particular document starting from a given position.
@@ -222,8 +222,7 @@ public boolean onMatch() {
222222
return matchSpace.equals(findWord);
223223
}
224224

225-
/** If we're on a match for the find word, replace it with the replace word.
226-
* Only executes in event thread.
225+
/** If we're on a match for the find word, replace it with the replace word. Only executes in event thread.
227226
* @return true if we're on a match; false otherwise
228227
*/
229228
public boolean replaceCurrent() {
@@ -259,22 +258,20 @@ public boolean replaceCurrent() {
259258
*/
260259
public void setSelection(MovingDocumentRegion s) { _selectionRegion = s; }
261260

262-
/** Replaces all occurrences of the find word with the replace word in the current document of in all documents
263-
* depending the value of the machine register _searchAllDocuments.
264-
* @return the number of replacements
265-
*/
261+
/** Alternative interface for the private method replaceAll(...) */
266262
public int replaceAll() { return replaceAll(_searchAllDocuments, _searchSelectionOnly); }
267263

268-
/** Replaces all occurences of the find word with the replace word in the current document of in all documents or in
269-
* the current selection depending the value of the flag searchAll
270-
* @param searchAll true if we should search for occurrences in all documents
271-
* @param searchSelectionOnly true if we should only search in the current selection of documents
264+
/** Replaces all occurences of the find word with the replace word
265+
* (i) in the current document or
266+
* (ii) in all documents or
267+
* (iii) in the current selection of the current document
268+
* (depending the value of the flags searchAll and searchSelectionOnly)
272269
* @return the number of replacements
273270
*/
274271
private int replaceAll(boolean searchAll, boolean searchSelectionOnly) {
275272

276273
assert EventQueue.isDispatchThread();
277-
274+
278275
if (searchAll) {
279276
int count = 0; // the number of replacements done so far
280277
int n = _docIterator.getDocumentCount();
@@ -283,15 +280,15 @@ private int replaceAll(boolean searchAll, boolean searchSelectionOnly) {
283280
count += _replaceAllInCurrentDoc(false);
284281
_doc = _docIterator.getNextDocument(_doc, _frame);
285282

286-
if (_doc==null) break;
283+
if (_doc == null) break;
287284
}
288285

289286
// update display (adding "*") in navigatgorPane
290287
_model.getDocumentNavigator().repaint();
291288

292289
return count;
293290
}
294-
else if(searchSelectionOnly) {
291+
else if (searchSelectionOnly) {
295292
int count = 0;
296293
count += _replaceAllInCurrentDoc(searchSelectionOnly);
297294
return count;
@@ -339,8 +336,9 @@ private int _replaceAllInCurrentDoc(boolean searchSelectionOnly) {
339336
return count;
340337
}
341338

342-
/** Processes all occurences of the find word with the replace word in the current document or in all documents
343-
* depending the value of the machine register _searchAllDocuments.
339+
/** Processes all occurrences of the find word with the replace word in the current document or in all documents
340+
* depending the values of fields _searchAllDocuments and _searchSelectionOnly and parameter region. Assumes that
341+
* findAction does not modify the document it processes. Only executes in event thread.
344342
* @param findAction action to perform on the occurrences; input is the FindResult, output is ignored
345343
* @param region the selection region
346344
* @return the number of processed occurrences
@@ -350,28 +348,28 @@ public int processAll(Runnable1<FindResult> findAction, MovingDocumentRegion reg
350348
assert EventQueue.isDispatchThread();
351349

352350
_selectionRegion = region;
353-
return processAll(findAction, _searchAllDocuments, _searchSelectionOnly);
351+
return processAll(findAction);
354352
}
355353

356-
/** Processes all occurences of the find word with the replace word in the
357-
* current document or in all documents depending the value of the flag
358-
* searchAll. Assumes that findAction does not modify the document it
359-
* processes. Only executes in event thread.
360-
* @param findAction action to perform on the occurrences; input is the FindResult, output is ignored
361-
* @param searchAll true if we should search for occurrences in all documents
362-
* @param searchSelectionOnly true if we should only search in the current selection of documents
363-
* @return the number of replacements
364-
*/
365-
private int processAll(Runnable1<FindResult> findAction, boolean searchAll, boolean searchSelectionOnly) {
354+
/** Processes all occurences of the find word with the replace word in the current document or in all documents
355+
* depending the value of fields _searchAllDocuments, _searchSelectionOnly, _selectionRegion. Assumes that
356+
* findAction does not modify the document it processes. Only executes in event thread.
357+
* @param findAction action to perform on the occurrences; input is the FindResult, output is ignored
358+
* @param searchAll true if we should search for occurrences in all documents
359+
* @param searchSelectionOnly true if we should only search in the current selection of document
360+
* @return the number of replacements
361+
*/
362+
private int processAll(Runnable1<FindResult> findAction) {
366363

367364
assert EventQueue.isDispatchThread();
368365

369-
if (searchAll) {
366+
if (_searchAllDocuments) {
370367
int count = 0; // the number of replacements done so far
371368
final int n = _docIterator.getDocumentCount();
372369
for (int i = 0; i < n; i++) {
373370
// process all in the rest of the documents
374-
count += _processAllInCurrentDoc(findAction, false);
371+
_searchSelectionOnly = false; // force _searchSelectionOnly to be false
372+
count += _processAllInCurrentDoc(findAction);
375373
_doc = _docIterator.getNextDocument(_doc, _frame);
376374

377375
if(_doc == null) break;
@@ -382,12 +380,12 @@ private int processAll(Runnable1<FindResult> findAction, boolean searchAll, bool
382380

383381
return count;
384382
}
385-
else if(searchSelectionOnly) {
383+
else if (_searchSelectionOnly) {
386384
int count = 0;
387-
count += _processAllInCurrentDoc(findAction, searchSelectionOnly);
385+
count += _processAllInCurrentDoc(findAction);
388386
return count;
389387
}
390-
else return _processAllInCurrentDoc(findAction, false);
388+
else return _processAllInCurrentDoc(findAction);
391389
}
392390

393391
/** Processes all occurences of _findWord in _doc. Never processes other documents. Starts at the beginning or the
@@ -399,14 +397,13 @@ else if(searchSelectionOnly) {
399397
* Only executes in event thread. Assumes (i) this has exclusive access to _doc (since it only runs in event thread)
400398
* and (ii) findAction does not modify _doc.
401399
* @param findAction action to perform on the occurrences; input is the FindResult, output is ignored
402-
* @param searchSelectionOnly true if we should only search in the current selection of documents
403400
* @return the number of replacements
404401
*/
405-
private int _processAllInCurrentDoc(Runnable1<FindResult> findAction, boolean searchSelectionOnly) {
402+
private int _processAllInCurrentDoc(Runnable1<FindResult> findAction) {
406403

407404
assert EventQueue.isDispatchThread();
408405

409-
if (!searchSelectionOnly) {
406+
if (!_searchSelectionOnly) {
410407
_selectionRegion = new MovingDocumentRegion(_doc, 0, _doc.getLength(), _doc._getLineStartPos(0),
411408
_doc._getLineEndPos(_doc.getLength()));
412409
}
@@ -416,7 +413,7 @@ private int _processAllInCurrentDoc(Runnable1<FindResult> findAction, boolean se
416413
int count = 0;
417414
FindResult fr = findNext(false); // find next match in current doc
418415

419-
while (! fr.isWrapped() && fr.getFoundOffset() <= _selectionRegion.getEndOffset()) {
416+
while (!fr.isWrapped() && fr.getFoundOffset() <= _selectionRegion.getEndOffset()) {
420417
findAction.run(fr);
421418
count++;
422419
fr = findNext(false); // find next match in current doc

drjava/src/edu/rice/cs/drjava/model/GlobalModelCompileTest.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ public void run() {
7070
}
7171
});
7272
listener.waitCompileDone();
73-
if (_model.getCompilerModel().getNumErrors() > 0) {
74-
fail("compile failed: " + getCompilerErrorString());
73+
if (_model.getCompilerModel().getNumErrors() != 1) {
74+
fail("Compilation Failure should generate one error: " + getCompilerErrorString());
7575
}
76-
assertCompileErrorsPresent("compile should succeed", false);
76+
assertCompileErrorsPresent("compile should succeed", true);
7777
listener.checkCompileOccurred();
7878
_model.removeListener(listener);
7979
_log.log("testCompileAllWithNoFiles complete");
@@ -88,13 +88,14 @@ public void run() {
8888
public void testCompileResetsInteractions() throws BadLocationException, IOException, InterruptedException,
8989
EditDocumentException {
9090

91-
// System.err.println("Starting testCompileResetsInteractions");
91+
_log.log("Starting testCompileResetsInteractions");
9292

9393
OpenDefinitionsDocument doc = setupDocument(FOO_TEXT);
9494
final File file = new File(_tempDir, "DrJavaTestFoo.java");
9595
saveFile(doc, new FileSelector(file));
9696

9797
// Use the interpreter so resetInteractions is not optimized to a no-op
98+
_log.log("Interpreting 0");
9899
interpret("0");
99100

100101
CompileShouldSucceedListener listener = new CompileShouldSucceedListener();
@@ -107,6 +108,7 @@ public void run() {
107108
}
108109
});
109110
listener.waitCompileDone();
111+
_log.log("Compilation complete");
110112

111113
if (_model.getCompilerModel().getNumErrors() > 0) {
112114
// System.err.println("Compile failed");

drjava/src/edu/rice/cs/drjava/model/JDKDescriptor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public static boolean exists(File jarOrDir, String... fileNames) {
207207
JarFile jf = new JarFile(jarOrDir);
208208
for (String fn: fileNames) {
209209

210-
if (jf.getJarEntry(fn)==null) {
210+
if (jf.getJarEntry(fn) == null) {
211211
jf.close();
212212
return false;
213213
}

drjava/src/edu/rice/cs/drjava/model/JarJDKToolsLibrary.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,7 @@ private static void addIfFile(File f, Set<JDKDescriptor> cs, Map<? super File,Se
736736
f = IOUtil.attemptCanonicalFile(f);
737737
if (IOUtil.attemptIsFile(f)) {
738738
Set<JDKDescriptor> set = map.get(f);
739-
if (set==null) {
739+
if (set == null) {
740740
set = new LinkedHashSet<JDKDescriptor>();
741741
map.put(f, set);
742742
}

0 commit comments

Comments
 (0)