Corrects NPE on working directory check and uses UUID for secondary ID. Change-Id: I1ff55053a576fbfc83a013cf4a557e10c1ca64a6
diff --git a/com.google.eclipse.elt.view/src/com/google/eclipse/elt/view/ui/TerminalView.java b/com.google.eclipse.elt.view/src/com/google/eclipse/elt/view/ui/TerminalView.java index fa8c751..651fa15 100644 --- a/com.google.eclipse.elt.view/src/com/google/eclipse/elt/view/ui/TerminalView.java +++ b/com.google.eclipse.elt.view/src/com/google/eclipse/elt/view/ui/TerminalView.java
@@ -8,34 +8,48 @@ */ package com.google.eclipse.elt.view.ui; -import static org.eclipse.core.runtime.Path.fromOSString; -import static org.eclipse.core.runtime.Status.OK_STATUS; -import static org.eclipse.jface.resource.JFaceResources.TEXT_FONT; -import static org.eclipse.jface.window.Window.OK; -import static org.eclipse.ui.IWorkbenchPage.VIEW_ACTIVATE; -import static com.google.eclipse.elt.view.Activator.*; -import static com.google.eclipse.elt.view.ImageKeys.*; -import static com.google.eclipse.elt.view.preferences.ColorsAndFontsPreferences.*; -import static com.google.eclipse.elt.view.preferences.GeneralPreferences.*; -import static com.google.eclipse.elt.view.ui.Messages.*; -import static com.google.eclipse.elt.view.util.Platform.userHomeDirectory; +import com.google.eclipse.elt.emulator.control.ITerminalListener; +import com.google.eclipse.elt.emulator.provisional.api.TerminalState; +import com.google.eclipse.elt.view.Activator; +import com.google.eclipse.elt.view.ImageKeys; +import com.google.eclipse.elt.view.connector.LifeCycleListener; +import com.google.eclipse.elt.view.preferences.AbstractPreferencesChangeListener; +import com.google.eclipse.elt.view.preferences.ColorsAndFontsPreferences; +import com.google.eclipse.elt.view.preferences.GeneralPreferences; -import org.eclipse.core.runtime.*; -import org.eclipse.jface.action.*; -import org.eclipse.jface.dialogs.*; +import org.eclipse.core.runtime.IPath; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Path; +import org.eclipse.core.runtime.Status; +import org.eclipse.jface.action.Action; +import org.eclipse.jface.action.IToolBarManager; +import org.eclipse.jface.action.Separator; +import org.eclipse.jface.dialogs.IInputValidator; +import org.eclipse.jface.dialogs.InputDialog; import org.eclipse.jface.resource.JFaceResources; -import org.eclipse.jface.util.*; +import org.eclipse.jface.util.IPropertyChangeListener; +import org.eclipse.jface.util.PropertyChangeEvent; +import org.eclipse.jface.window.Window; import org.eclipse.swt.graphics.Font; -import org.eclipse.swt.widgets.*; -import org.eclipse.ui.*; -import org.eclipse.ui.contexts.*; +import org.eclipse.swt.widgets.Composite; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.ui.IMemento; +import org.eclipse.ui.ISaveablePart2; +import org.eclipse.ui.IViewPart; +import org.eclipse.ui.IViewSite; +import org.eclipse.ui.IWorkbenchPage; +import org.eclipse.ui.IWorkbenchPartSite; +import org.eclipse.ui.PartInitException; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.contexts.IContextActivation; +import org.eclipse.ui.contexts.IContextService; import org.eclipse.ui.part.ViewPart; import org.eclipse.ui.progress.UIJob; -import com.google.eclipse.elt.emulator.control.ITerminalListener; -import com.google.eclipse.elt.emulator.provisional.api.TerminalState; -import com.google.eclipse.elt.view.connector.LifeCycleListener; -import com.google.eclipse.elt.view.preferences.AbstractPreferencesChangeListener; +import java.io.File; +import java.util.UUID; /** * @author alruiz@google.com (Alex Ruiz) @@ -70,8 +84,6 @@ openNewTerminalView(workingDirectory); } - private static int freshTerminalId = 0; - /** * Opens a new {@link TerminalView}. * @@ -79,17 +91,18 @@ */ private static void openNewTerminalView(IPath workingDirectory) { IWorkbenchPage page = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage(); - IPath safeWorkingDirectory = (workingDirectory != null) ? workingDirectory : userHomeDirectory(); + IPath safeWorkingDirectory = + (workingDirectory != null) ? workingDirectory : getDefaultWorkingDirectory(); try { String directoryName = safeWorkingDirectory.lastSegment(); // Use of a freshTerminalId to force freshness of the identifier. - String secondaryId = Integer.toString(freshTerminalId); - freshTerminalId++; - TerminalView view = (TerminalView) page.showView(VIEW_ID, secondaryId, VIEW_ACTIVATE); + String secondaryId = UUID.randomUUID().toString(); + TerminalView view = + (TerminalView) page.showView(VIEW_ID, secondaryId, IWorkbenchPage.VIEW_ACTIVATE); view.setPartName(directoryName); view.open(safeWorkingDirectory); } catch (PartInitException e) { - log("Unable to create Terminal View", e); + Activator.log("Unable to create Terminal View", e); } } @@ -101,13 +114,37 @@ @Override public void saveState(IMemento memento) { saveState(memento, SCROLL_LOCK_ENABLED, String.valueOf(terminalWidget.isScrollLockEnabled())); saveState(memento, TITLE_STATE_TYPE, getPartName()); - saveState(memento, WORKING_DIRECTORY_STATE_TYPE, workingDirectory.toOSString()); + if (workingDirectory != null) { + saveState(memento, WORKING_DIRECTORY_STATE_TYPE, workingDirectory.toOSString()); + } } private void saveState(IMemento memento, String type, String data) { IMemento child = memento.createChild(type); child.putTextData(data); } + + /** + * Returns a non-{@code null} directory to be used as the default working directory if one isn't + * explicitly provided. + */ + private static IPath getDefaultWorkingDirectory() { + File homeDirectory = new File(System.getProperty("user.home")); + if (homeDirectory.isDirectory()) { + return Path.fromOSString(homeDirectory.getAbsolutePath()); + } + File currentDirectory = new File(System.getProperty("user.dir")); + if (currentDirectory.isDirectory()) { + return Path.fromOSString(currentDirectory.getAbsolutePath()); + } + // If all else fails, pick the first root we can find. + for (File root : File.listRoots()) { + if (root.isDirectory()) { + return Path.fromOSString(root.getAbsolutePath()); + } + } + throw new IllegalStateException("Could not find valid working directory."); + } @Override public void createPartControl(Composite parent) { terminalWidget = new TerminalWidget(parent, getViewSite()); @@ -141,14 +178,14 @@ updateUsageOfBlinkingCursor(); } }; - preferenceStore().addPropertyChangeListener(preferencesChangeListener); + Activator.preferenceStore().addPropertyChangeListener(preferencesChangeListener); updateBufferLineCount(); updateColors(); updateUsageOfBlinkingCursor(); textFontChangeListener = new IPropertyChangeListener() { @Override public void propertyChange(PropertyChangeEvent event) { - if (TEXT_FONT.equals(event.getProperty())) { - if (!useCustomFont()) { + if (JFaceResources.TEXT_FONT.equals(event.getProperty())) { + if (!ColorsAndFontsPreferences.useCustomFont()) { setFont(JFaceResources.getTextFont()); } } @@ -159,7 +196,8 @@ setupToolBarActions(); IContextService contextService = contextService(); if (contextService != null) { - contextActivation = contextService.activateContext("com.google.eclipse.terminal.local.context.localTerminal"); + contextActivation = + contextService.activateContext("com.google.eclipse.terminal.local.context.localTerminal"); } if (savedState != null) { updateScrollLockUsingSavedState(); @@ -167,14 +205,15 @@ return; } if (viewSite.getSecondaryId() == null) { - setPartName(defaultViewTitle); - open(userHomeDirectory()); + setPartName(Messages.defaultViewTitle); + open(getDefaultWorkingDirectory()); } enableScrollLock(scrollLockAction.isChecked()); } private void closeViewOnExitIfPossible() { - if (closeViewOnExit() && terminalWidget != null && !terminalWidget.isDisposed()) { + if (GeneralPreferences.closeViewOnExit() && terminalWidget != null + && !terminalWidget.isDisposed()) { // must run in UI thread. forceClose = true; terminalWidget.getDisplay().asyncExec(new Runnable() { @@ -187,7 +226,8 @@ } private void updateColors() { - terminalWidget.setColors(background(), foreground()); + terminalWidget.setColors(ColorsAndFontsPreferences.background(), + ColorsAndFontsPreferences.foreground()); } private void updateFont() { @@ -195,12 +235,12 @@ } private void updateUsageOfBlinkingCursor() { - terminalWidget.setBlinkingCursor(useBlinkingCursor()); + terminalWidget.setBlinkingCursor(ColorsAndFontsPreferences.useBlinkingCursor()); } private Font terminalFont() { - if (useCustomFont()) { - return new Font(Display.getDefault(), customFontData()); + if (ColorsAndFontsPreferences.useCustomFont()) { + return new Font(Display.getDefault(), ColorsAndFontsPreferences.customFontData()); } return JFaceResources.getTextFont(); } @@ -210,7 +250,7 @@ } private void updateBufferLineCount() { - terminalWidget.setBufferLineCount(bufferLineCount()); + terminalWidget.setBufferLineCount(GeneralPreferences.bufferLineCount()); } private void setupToolBarActions() { @@ -241,8 +281,10 @@ String title = savedState(TITLE_STATE_TYPE); setPartName(title); String savedWorkingDirectory = savedState(WORKING_DIRECTORY_STATE_TYPE); - if (savedWorkingDirectory != null) { - open(fromOSString(savedWorkingDirectory)); + if (savedWorkingDirectory == null) { + open(getDefaultWorkingDirectory()); + } else { + open(Path.fromOSString(savedWorkingDirectory)); } } @@ -252,6 +294,7 @@ } private void open(IPath workingDirectory) { + // workingDirectory should be non-null. if (terminalWidget.isConnected()) { return; } @@ -264,7 +307,7 @@ UIJob job = new UIJob("Update terminal view title") { @Override public IStatus runInUIThread(IProgressMonitor monitor) { setPartName(value); - return OK_STATUS; + return Status.OK_STATUS; } }; job.schedule(); @@ -282,7 +325,7 @@ } } if (preferencesChangeListener != null) { - preferenceStore().removePropertyChangeListener(preferencesChangeListener); + Activator.preferenceStore().removePropertyChangeListener(preferencesChangeListener); } if (textFontChangeListener != null) { JFaceResources.getFontRegistry().removeListener(textFontChangeListener); @@ -311,7 +354,7 @@ } @Override public int promptToSaveOnClose() { - if (warnOnClose()) { + if (GeneralPreferences.warnOnClose()) { boolean close = WarnOnCloseDialog.open(terminalWidget.getShell()); if (!close) { return CANCEL; @@ -330,8 +373,8 @@ private class NewTerminalAction extends Action { NewTerminalAction() { - setImageDescriptor(imageDescriptor(NEW_TERMINAL)); - setText(newLocalTerminal); + setImageDescriptor(Activator.imageDescriptor(ImageKeys.NEW_TERMINAL)); + setText(Messages.newLocalTerminal); } @Override public void run() { @@ -341,9 +384,9 @@ private class ScrollLockAction extends Action { ScrollLockAction() { - super(scrollLock, AS_RADIO_BUTTON); + super(Messages.scrollLock, AS_RADIO_BUTTON); setChecked(false); - setImageDescriptor(imageDescriptor(SCROLL_LOCK)); + setImageDescriptor(Activator.imageDescriptor(ImageKeys.SCROLL_LOCK)); } @Override public void run() { @@ -354,14 +397,15 @@ private class ChangeViewNameAction extends Action { ChangeViewNameAction() { - setImageDescriptor(imageDescriptor(CHANGE_TITLE)); - setText(changeTerminalTitle); + setImageDescriptor(Activator.imageDescriptor(ImageKeys.CHANGE_TITLE)); + setText(Messages.changeTerminalTitle); } @Override public void run() { Shell shell = getViewSite().getShell(); final String currentTitle = getPartName(); - InputDialog input = new InputDialog(shell, enterTerminalTitleDialogTitle, enterTerminalTitlePrompt, currentTitle, + InputDialog input = new InputDialog(shell, Messages.enterTerminalTitleDialogTitle, + Messages.enterTerminalTitlePrompt, currentTitle, new IInputValidator() { @Override public String isValid(String newText) { if (newText == null || newText.isEmpty() || currentTitle.equals(newText)) { @@ -371,7 +415,7 @@ } }); input.setBlockOnOpen(true); - if (input.open() == OK) { + if (input.open() == Window.OK) { setPartName(input.getValue()); } }