Skip to content

Commit 54ea28b

Browse files
author
mgricken
committed
This commit fixes two bugs:
[ 1467537 ] NullPointerException After Close All Files/Open Folder [ 1466590 ] Cancel in "Save?" Dialogs on Quit The first bug was caused by File.listFiles returning null in case of an IO error. I looked for every use of File.listFiles and inserted a check for null. The second bug was mainly handled by doing a round of canAbandonFile calls that take care of saving and possibly cancelling. Only if all files were processed that way, all files were closed. The commit also removes the uses of getEnclosingClassName, which unfortunately is incorrect and slow. M src/edu/rice/cs/drjava/model/DummyGlobalModelListener.java M src/edu/rice/cs/drjava/model/debug/DocumentDebugAction.java M src/edu/rice/cs/drjava/model/GlobalModelTestCase.java M src/edu/rice/cs/drjava/model/DefaultGlobalModel.java M src/edu/rice/cs/drjava/model/definitions/DefinitionsDocumentTest.java M src/edu/rice/cs/drjava/model/definitions/DefinitionsDocument.java M src/edu/rice/cs/drjava/model/DefaultJavadocModel.java M src/edu/rice/cs/drjava/model/GlobalEventNotifier.java M src/edu/rice/cs/drjava/model/junit/DefaultJUnitModel.java M src/edu/rice/cs/drjava/model/GlobalModelListener.java M src/edu/rice/cs/drjava/model/GlobalModelJUnitTest.java M src/edu/rice/cs/drjava/model/DummyOpenDefDoc.java M src/edu/rice/cs/drjava/model/DummyGlobalModel.java M src/edu/rice/cs/drjava/model/AbstractGlobalModel.java M src/edu/rice/cs/drjava/model/SingleDisplayModel.java M src/edu/rice/cs/drjava/model/OpenDefinitionsDocument.java M src/edu/rice/cs/drjava/config/OptionConstants.java M src/edu/rice/cs/drjava/ui/MainFrame.java M src/edu/rice/cs/drjava/ui/JarOptionsDialog.java M src/edu/rice/cs/drjava/ui/config/ConfigFrame.java M src/edu/rice/cs/util/FileOps.java M src/edu/rice/cs/util/jar/JarBuilder.java git-svn-id: file:///tmp/test-svn/trunk@3760 fe72c1cf-3628-48e9-8b72-1c46755d3cff
1 parent 065b1e3 commit 54ea28b

22 files changed

+323
-122
lines changed

‎drjava/src/edu/rice/cs/drjava/config/OptionConstants.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,8 @@ public static ArrayList<String> evaluate() {
10961096
/** Whether to scan class files for auto-completion class names. */
10971097
public static final BooleanOption DIALOG_COMPLETE_SCAN_CLASS_FILES =
10981098
new BooleanOption("dialog.completefile.scan.class.files", Boolean.FALSE);
1099-
1099+
1100+
// Any lightweight parsing has been disabled until we have something that is beneficial and works better in the background.
11001101
/** Whether to perform light-weight parsing. */
11011102
public static final BooleanOption LIGHTWEIGHT_PARSING_ENABLED =
11021103
new BooleanOption("lightweight.parsing.enabled", Boolean.FALSE);

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

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333

3434
package edu.rice.cs.drjava.model;
3535

36+
import edu.rice.cs.util.Log;
37+
3638
import java.awt.Container;
3739
import java.awt.event.FocusEvent;
3840
import java.awt.event.FocusListener;
@@ -250,12 +252,13 @@ public void removeAuxiliaryFile(OpenDefinitionsDocument doc) {
250252
*/
251253
protected IDocumentNavigator<OpenDefinitionsDocument> _documentNavigator =
252254
new AWTContainerNavigatorFactory<OpenDefinitionsDocument>().makeListNavigator();
253-
254-
/** Light-weight parsing controller. */
255-
protected LightWeightParsingControl _parsingControl;
256255

257-
/** @return the parsing control */
258-
public LightWeightParsingControl getParsingControl() { return _parsingControl; }
256+
// Any lightweight parsing has been disabled until we have something that is beneficial and works better in the background.
257+
// /** Light-weight parsing controller. */
258+
// protected LightWeightParsingControl _parsingControl;
259+
//
260+
// /** @return the parsing control */
261+
// public LightWeightParsingControl getParsingControl() { return _parsingControl; }
259262

260263
// ----- CONSTRUCTORS -----
261264

@@ -688,9 +691,12 @@ public boolean accept(File parent, String name) {
688691
}
689692
});
690693

691-
for (File kid: fs) { cleanHelper(kid); }
694+
if (fs!=null) { // listFiles may return null if there's an IO error
695+
for (File kid: fs) { cleanHelper(kid); }
696+
}
692697

693-
if (f.listFiles().length == 0) f.delete();
698+
fs = f.listFiles(); // listFiles may return null if there's an IO error
699+
if ((fs!=null) && (fs.length == 0)) f.delete();
694700

695701
} else if (f.getName().endsWith(".class")) f.delete();
696702
}
@@ -712,7 +718,9 @@ public boolean accept(File parent, String name) {
712718
}
713719
});
714720

715-
for (File kid: fs) { getClassFilesHelper(kid, acc); }
721+
if (fs!=null) { // listFiles may return null if there's an IO error
722+
for (File kid: fs) { getClassFilesHelper(kid, acc); }
723+
}
716724

717725
} else if (f.getName().endsWith(".class")) acc.add(f);
718726
}
@@ -1521,13 +1529,6 @@ protected boolean closeFileHelper(OpenDefinitionsDocument doc) {
15211529
return false;
15221530
}
15231531

1524-
/** Similar to closeFileHelper except that saving cannot be cancelled. It is public for testing purposes. */
1525-
public void closeFileOnQuitHelper(OpenDefinitionsDocument doc) {
1526-
// System.err.println("closing " + doc);
1527-
doc.quitFile();
1528-
closeFileWithoutPrompt(doc);
1529-
}
1530-
15311532
/** Closes an open definitions document, without prompting to save if the document has been changed. Returns
15321533
* whether the file was successfully closed. NOTE: This method should not be called unless it can be
15331534
* absolutely known that the document being closed is not the active document. The closeFile() method in
@@ -1564,26 +1565,45 @@ public boolean closeFileWithoutPrompt(final OpenDefinitionsDocument doc) {
15641565
/** Closes all open documents without creating a new empty document. It cannot be cancelled by the user
15651566
* because it would leave the current project in an inconsistent state. Method is public for
15661567
* testing purposes.
1568+
* @param false if the user cancelled
15671569
*/
1568-
public void closeAllFilesOnQuit() {
1570+
public boolean closeAllFilesOnQuit() {
15691571

1570-
OpenDefinitionsDocument[] docs;
1571-
synchronized(_documentsRepos) { docs = _documentsRepos.toArray(new OpenDefinitionsDocument[0]); }
1572+
List<OpenDefinitionsDocument> docList;
1573+
synchronized(_documentsRepos) { docList = new ArrayList<OpenDefinitionsDocument> (_documentsRepos); }
15721574

1573-
for (OpenDefinitionsDocument doc : docs) {
1574-
closeFileOnQuitHelper(doc); // modifies _documentsRepos
1575+
// first see if the user wants to cancel on any of them
1576+
boolean canClose = true;
1577+
for (OpenDefinitionsDocument doc : docList) {
1578+
if (!doc.canAbandonFile()) { canClose = false; break; }
15751579
}
1580+
1581+
if (!canClose) { return false; } // the user did want to cancel
1582+
1583+
// user did not want to cancel, close all of them
1584+
// All files are being closed, create a new file before starting in order to have
1585+
// a potentially active file that is not in the list of closing files.
1586+
newFile();
1587+
1588+
// Set the active document to the document just after the last document or the document just before the
1589+
// first document in docList. A new file does not appear in docList.
1590+
_ensureNotActive(docList);
1591+
1592+
// Close the files in docList.
1593+
for (OpenDefinitionsDocument doc : docList) { closeFileWithoutPrompt(doc); }
1594+
1595+
return true;
15761596
}
15771597

15781598
/** Exits the program. Quits regardless of whether all documents are successfully closed. */
15791599
public void quit() {
15801600
try {
1581-
closeAllFilesOnQuit();
1601+
if (!closeAllFilesOnQuit()) return;
15821602
// Utilities.show("Closed all files");
15831603
dispose(); // kills the interpreter
1604+
System.exit(0);
15841605
}
1585-
catch(Throwable t) { /* do nothing */ }
1586-
finally { System.exit(0); }
1606+
catch(Throwable t) { System.exit(0); /* exit anyway */ }
15871607
}
15881608

15891609
/** Prepares this model to be thrown away. Never called in practice outside of quit(), except in tests.
@@ -2684,10 +2704,12 @@ public boolean canAbandonFile() {
26842704

26852705
/** Fires the quit(File) event if isModifiedSinceSave() is true. The quitFile() event asks the user if the
26862706
* the file should be saved before quitting.
2707+
* @return true if quitting should continue, false if the user cancelled
26872708
*/
2688-
public void quitFile() {
2689-
if (isModifiedSinceSave() || (_file != null && !_file.exists() && _cacheAdapter.isReady()))
2690-
_notifier.quitFile(this);
2709+
public boolean quitFile() {
2710+
if (isModifiedSinceSave() || (_file != null && !_file.exists() && _cacheAdapter.isReady())) {
2711+
return _notifier.quitFile(this);
2712+
} else { return true; }
26912713
}
26922714

26932715
/** Moves the definitions document to the given line, and returns the resulting character position.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,8 @@ public void optionChanged(OptionEvent<String> oe) {
219219
// Perhaps do this in another thread to allow startup to continue...
220220
_jvm.startInterpreterJVM();
221221

222-
_parsingControl = new DefaultLightWeightParsingControl(this);
222+
// Any lightweight parsing has been disabled until we have something that is beneficial and works better in the background.
223+
// _parsingControl = new DefaultLightWeightParsingControl(this);
223224
}
224225

225226

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,9 @@ private void _javadocAllWorker(File destDirFile, FileSaveSelector saver, String
235235
// But don't do it if we've already done it for this directory.
236236
defaultRoots.add(sourceRoot);
237237
File[] javaFiles = sourceRoot.listFiles(FileOps.JAVA_FILE_FILTER);
238-
for (File f: javaFiles) { docUnits.add(f.getAbsolutePath());}
238+
if (javaFiles!=null) { // listFiles may return null if there's an IO error
239+
for (File f: javaFiles) { docUnits.add(f.getAbsolutePath());}
240+
}
239241
}
240242
}
241243
else {

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -526,9 +526,10 @@ public boolean hasModifiedDocuments() {
526526
public boolean hasUntitledDocuments() {
527527
throw new UnsupportedOperationException("Tried to call hasUntitliedDocuments on a Dummy!");
528528
}
529-
530-
/** @return the parsing control */
531-
public LightWeightParsingControl getParsingControl() {
532-
throw new UnsupportedOperationException("Tried to call getParsingControl on a Dummy!");
533-
}
529+
530+
// Any lightweight parsing has been disabled until we have something that is beneficial and works better in the background.
531+
// /** @return the parsing control */
532+
// public LightWeightParsingControl getParsingControl() {
533+
// throw new UnsupportedOperationException("Tried to call getParsingControl on a Dummy!");
534+
// }
534535
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,9 @@ public void classFileError(ClassFileError e) { }
219219
/** Called to ask the listener if it is OK to abandon the current document. */
220220
public boolean canAbandonFile(OpenDefinitionsDocument doc) { return true; }
221221

222-
/** Called to ask the listener if the document should be saved before quitting. */
223-
public void quitFile(OpenDefinitionsDocument doc) { }
222+
/** Called to ask the listener if the document should be saved before quitting.
223+
* @return true if quitting should continue, false if the user cancelled */
224+
public boolean quitFile(OpenDefinitionsDocument doc) { return true; }
224225

225226
/** Called to ask the listener if it is OK to replace the current document by a newer version on disk. */
226227
public boolean shouldRevertFile(OpenDefinitionsDocument doc) { return true; }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public boolean saveFile(FileSaveSelector com) throws IOException {
9090

9191
public boolean canAbandonFile() { throw new UnsupportedOperationException("Dummy method"); }
9292

93-
public void quitFile() { throw new UnsupportedOperationException("Dummy method"); }
93+
public boolean quitFile() { throw new UnsupportedOperationException("Dummy method"); }
9494

9595
public void setCurrentLocation(int location) { throw new UnsupportedOperationException("Dummy method"); }
9696

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,13 +231,16 @@ public boolean canAbandonFile(OpenDefinitionsDocument doc) {
231231
finally { _lock.endRead(); }
232232
}
233233

234-
/** Called to ask the listeners save the file before quitting at the user's option. */
235-
public void quitFile(OpenDefinitionsDocument doc) {
234+
/** Called to ask the listeners save the file before quitting at the user's option.
235+
* @return true if quitting should continue, false if the user cancelled */
236+
public boolean quitFile(OpenDefinitionsDocument doc) {
236237
_lock.startRead();
237238
try {
238-
for (GlobalModelListener l: _listeners) { l.quitFile(doc); }
239+
// if one of the listeners returns false (=user cancelled), abort
240+
for (GlobalModelListener l: _listeners) { if (!l.quitFile(doc)) return false; }
239241
}
240242
finally { _lock.endRead(); }
243+
return true;
241244
}
242245

243246
/** Called to ask the listeners if it is OK to revert the current document to the version saved on disk. */

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,8 @@ public void testUnsavedAndUnCompiledChanges() throws Exception {
481481

482482
System.out.println("Untitled file is named: " + untitled.getName());
483483

484-
_model.closeFileOnQuitHelper(untitled);
484+
untitled.quitFile();
485+
_model.closeFileWithoutPrompt(untitled);
485486

486487
// set up test listener for compile command; automatically checks that compilation is performed
487488
JUnitTestListener listener = new JUnitCompileBeforeTestListener();

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,9 @@ public interface GlobalModelListener extends InteractionsListener,
7070
/** Called to ask the listener if it is OK to abandon the current document. */
7171
public boolean canAbandonFile(OpenDefinitionsDocument doc);
7272

73-
/** Called to ask the listener if this document should be saved before quitting. */
74-
public void quitFile(OpenDefinitionsDocument doc);
73+
/** Called to ask the listener if this document should be saved before quitting.
74+
* @return true if quitting should continue, false if the user cancelled */
75+
public boolean quitFile(OpenDefinitionsDocument doc);
7576

7677
/** Called to ask the listener if it is OK to revert the current document to the version saved on disk. */
7778
public boolean shouldRevertFile(OpenDefinitionsDocument doc);

0 commit comments

Comments
 (0)