From beb0263b8400958bfca4c8fd103fcdf350770955 Mon Sep 17 00:00:00 2001 From: Ben Fry Date: Mon, 18 Feb 2013 13:49:47 -0500 Subject: [PATCH] further code cleanup and refactoring --- .../app/contrib/ContributionListPanel.java | 16 ++- .../app/contrib/ContributionListing.java | 127 ++++++------------ .../app/contrib/ContributionManager.java | 53 +++++++- .../contrib/ContributionManagerDialog.java | 17 +-- .../app/contrib/ContributionPanel.java | 9 +- .../app/contrib/ProgressMonitor.java | 1 - todo.txt | 4 + 7 files changed, 123 insertions(+), 104 deletions(-) diff --git a/app/src/processing/app/contrib/ContributionListPanel.java b/app/src/processing/app/contrib/ContributionListPanel.java index 500dd7e79..e25537cf7 100644 --- a/app/src/processing/app/contrib/ContributionListPanel.java +++ b/app/src/processing/app/contrib/ContributionListPanel.java @@ -34,6 +34,10 @@ import java.awt.*; import processing.app.Base; +// The "Scrollable" implementation and its methods here take care of preventing +// the scrolling area from running exceptionally slowly. Not sure why they're +// necessary in the first place, however; seems like odd behavior. + public class ContributionListPanel extends JPanel implements Scrollable, ContributionChangeListener { static public final String DELETION_MESSAGE = @@ -54,10 +58,10 @@ public class ContributionListPanel extends JPanel implements Scrollable, Contrib public void hyperlinkUpdate(HyperlinkEvent e) { } }; - protected ContributionPanel selectedPanel; + private ContributionPanel selectedPanel; // protected JPanel statusPlaceholder; - protected StatusPanel status; - ContributionFilter filter; + private StatusPanel status; + private ContributionFilter filter; // private ContributionListing contribListing; private ContributionListing contribListing = ContributionListing.getInstance(); @@ -222,6 +226,11 @@ public class ContributionListPanel extends JPanel implements Scrollable, Contrib requestFocusInWindow(); } } + + + protected ContributionPanel getSelectedPanel() { + return selectedPanel; + } /** @@ -298,7 +307,6 @@ public class ContributionListPanel extends JPanel implements Scrollable, Contrib */ public int getScrollableUnitIncrement(Rectangle visibleRect, int orientation, int direction) { if (orientation == SwingConstants.VERTICAL) { - int lastHeight = 0, height = 0; int bottomOfScrollArea = visibleRect.y + visibleRect.height; diff --git a/app/src/processing/app/contrib/ContributionListing.java b/app/src/processing/app/contrib/ContributionListing.java index 9b10bb475..2388b8c9c 100644 --- a/app/src/processing/app/contrib/ContributionListing.java +++ b/app/src/processing/app/contrib/ContributionListing.java @@ -86,16 +86,11 @@ public class ContributionListing { } - public Comparator getComparator() { - return nameComparator; - } - - /** * Adds the installed libraries to the listing of libraries, replacing any * pre-existing libraries by the same name as one in the list. */ - public void updateInstalledList(List installedContributions) { + protected void updateInstalledList(List installedContributions) { for (Contribution contribution : installedContributions) { Contribution preexistingContribution = getContribution(contribution); if (preexistingContribution != null) { @@ -107,7 +102,7 @@ public class ContributionListing { } - public void replaceContribution(Contribution oldLib, Contribution newLib) { + protected void replaceContribution(Contribution oldLib, Contribution newLib) { if (oldLib == null || newLib == null) { return; } @@ -132,7 +127,7 @@ public class ContributionListing { } - public void addContribution(Contribution contribution) { + private void addContribution(Contribution contribution) { if (librariesByCategory.containsKey(contribution.getCategory())) { List list = librariesByCategory.get(contribution.getCategory()); list.add(contribution); @@ -149,7 +144,7 @@ public class ContributionListing { } - public void removeContribution(Contribution info) { + protected void removeContribution(Contribution info) { if (librariesByCategory.containsKey(info.getCategory())) { librariesByCategory.get(info.getCategory()).remove(info); } @@ -158,18 +153,18 @@ public class ContributionListing { } - public Contribution getContribution(Contribution contribution) { - for (Contribution preexistingContribution : allContributions) { - if (preexistingContribution.getName().equals(contribution.getName()) - && preexistingContribution.getType() == contribution.getType()) { - return preexistingContribution; + private Contribution getContribution(Contribution contribution) { + for (Contribution c : allContributions) { + if (c.getName().equals(contribution.getName()) && + c.getType() == contribution.getType()) { + return c; } } return null; } - public AvailableContribution getAvailableContribution(Contribution info) { + protected AvailableContribution getAvailableContribution(Contribution info) { for (AvailableContribution advertised : advertisedContributions) { if (advertised.getType() == info.getType() && advertised.getName().equals(info.getName())) { @@ -180,7 +175,7 @@ public class ContributionListing { } - public Set getCategories(ContributionFilter filter) { + protected Set getCategories(ContributionFilter filter) { Set outgoing = new HashSet(); Set categorySet = librariesByCategory.keySet(); @@ -200,20 +195,20 @@ public class ContributionListing { } - public List getAllContributions() { - return new ArrayList(allContributions); - } +// public List getAllContributions() { +// return new ArrayList(allContributions); +// } - public List getLibararies(String category) { - ArrayList libinfos = - new ArrayList(librariesByCategory.get(category)); - Collections.sort(libinfos, nameComparator); - return libinfos; - } +// public List getLibararies(String category) { +// ArrayList libinfos = +// new ArrayList(librariesByCategory.get(category)); +// Collections.sort(libinfos, nameComparator); +// return libinfos; +// } - public List getFilteredLibraryList(String category, List filters) { + protected List getFilteredLibraryList(String category, List filters) { ArrayList filteredList = new ArrayList(allContributions); Iterator it = filteredList.iterator(); @@ -234,7 +229,7 @@ public class ContributionListing { } - public boolean matches(Contribution contrib, String filter) { + private boolean matches(Contribution contrib, String filter) { int colon = filter.indexOf(":"); if (colon != -1) { String isText = filter.substring(0, colon); @@ -261,11 +256,10 @@ public class ContributionListing { || contrib.getParagraph() != null && contrib.getParagraph().toLowerCase().matches(filter) || contrib.getCategory() != null && contrib.getCategory().toLowerCase().matches(filter) || contrib.getName() != null && contrib.getName().toLowerCase().matches(filter); - } - public boolean isProperty(String property) { + private boolean isProperty(String property) { return property.startsWith("updat") || property.startsWith("upgrad") || property.startsWith("instal") && !property.startsWith("installabl") || property.equals("tool") || property.startsWith("lib") @@ -273,9 +267,11 @@ public class ContributionListing { } - /** Returns true if the contribution fits the given property, false otherwise. - * If the property is invalid, returns false. */ - public boolean hasProperty(Contribution contrib, String property) { + /** + * Returns true if the contribution fits the given property, false otherwise. + * If the property is invalid, returns false. + */ + private boolean hasProperty(Contribution contrib, String property) { // update, updates, updatable, upgrade if (property.startsWith("updat") || property.startsWith("upgrad")) { return hasUpdates(contrib); @@ -323,7 +319,7 @@ public class ContributionListing { } - public void addContributionListener(ContributionChangeListener listener) { + protected void addContributionListener(ContributionChangeListener listener) { for (Contribution contrib : allContributions) { listener.contributionAdded(contrib); } @@ -331,21 +327,23 @@ public class ContributionListing { } - public void removeContributionListener(ContributionChangeListener listener) { + /* + private void removeContributionListener(ContributionChangeListener listener) { listeners.remove(listener); } - public ArrayList getContributionListeners() { + private ArrayList getContributionListeners() { return new ArrayList(listeners); } + */ /** * Starts a new thread to download the advertised list of contributions. * Only one instance will run at a time. */ - public void downloadAvailableList(ProgressMonitor pm) { + protected void downloadAvailableList(ProgressMonitor pm) { final ProgressMonitor progressMonitor = (pm != null) ? pm : new NullProgressMonitor(); @@ -362,7 +360,7 @@ public class ContributionListing { } if (!progressMonitor.isFinished()) { - download(url, listingFile, progressMonitor); + ContributionManager.download(url, listingFile, progressMonitor); if (!progressMonitor.isCanceled() && !progressMonitor.isError()) { hasDownloadedLatestList = true; setAdvertisedList(listingFile); @@ -374,56 +372,6 @@ public class ContributionListing { } - /** - * Blocks until the file is downloaded or an error occurs. - * Returns true if the file was successfully downloaded, false otherwise. - * - * @param source - * the URL of the file to download - * @param dest - * the file on the local system where the file will be written. This - * must be a file (not a directory), and must already exist. - * @param progress - * @throws FileNotFoundException - * if an error occurred downloading the file - */ - static boolean download(URL source, File dest, ProgressMonitor progress) { - boolean success = false; - try { -// System.out.println("downloading file " + source); - URLConnection conn = source.openConnection(); - conn.setConnectTimeout(1000); - conn.setReadTimeout(5000); - - // TODO this is often -1, may need to set progress to indeterminate - int fileSize = conn.getContentLength(); -// System.out.println("file size is " + fileSize); - progress.startTask("Downloading", fileSize); - - InputStream in = conn.getInputStream(); - FileOutputStream out = new FileOutputStream(dest); - - byte[] b = new byte[8192]; - int amount; - int total = 0; - while (!progress.isCanceled() && (amount = in.read(b)) != -1) { - out.write(b, 0, amount); - total += amount; - progress.setProgress(total); - } - out.flush(); - out.close(); - success = true; - - } catch (IOException ioe) { - progress.error(ioe); - ioe.printStackTrace(); - } - progress.finished(); - return success; - } - - boolean hasUpdates() { for (Contribution info : allContributions) { if (hasUpdates(info)) { @@ -526,6 +474,11 @@ public class ContributionListing { // } + public Comparator getComparator() { + return nameComparator; + } + + static Comparator nameComparator = new Comparator() { public int compare(Contribution o1, Contribution o2) { return o1.getName().toLowerCase().compareTo(o2.getName().toLowerCase()); diff --git a/app/src/processing/app/contrib/ContributionManager.java b/app/src/processing/app/contrib/ContributionManager.java index e5b116659..7827e2634 100644 --- a/app/src/processing/app/contrib/ContributionManager.java +++ b/app/src/processing/app/contrib/ContributionManager.java @@ -23,6 +23,7 @@ package processing.app.contrib; import java.io.*; import java.net.URL; +import java.net.URLConnection; import processing.app.Base; import processing.app.Editor; @@ -36,6 +37,56 @@ public class ContributionManager { } + /** + * Blocks until the file is downloaded or an error occurs. + * Returns true if the file was successfully downloaded, false otherwise. + * + * @param source + * the URL of the file to download + * @param dest + * the file on the local system where the file will be written. This + * must be a file (not a directory), and must already exist. + * @param progress + * @throws FileNotFoundException + * if an error occurred downloading the file + */ + static boolean download(URL source, File dest, ProgressMonitor progress) { + boolean success = false; + try { +// System.out.println("downloading file " + source); + URLConnection conn = source.openConnection(); + conn.setConnectTimeout(1000); + conn.setReadTimeout(5000); + + // TODO this is often -1, may need to set progress to indeterminate + int fileSize = conn.getContentLength(); +// System.out.println("file size is " + fileSize); + progress.startTask("Downloading", fileSize); + + InputStream in = conn.getInputStream(); + FileOutputStream out = new FileOutputStream(dest); + + byte[] b = new byte[8192]; + int amount; + int total = 0; + while (!progress.isCanceled() && (amount = in.read(b)) != -1) { + out.write(b, 0, amount); + total += amount; + progress.setProgress(total); + } + out.flush(); + out.close(); + success = true; + + } catch (IOException ioe) { + progress.error(ioe); + ioe.printStackTrace(); + } + progress.finished(); + return success; + } + + /** * Non-blocking call to download and install a contribution in a new thread. * @@ -63,7 +114,7 @@ public class ContributionManager { contribZip.setWritable(true); // necessary? try { - ContributionListing.download(url, contribZip, downloadProgress); + download(url, contribZip, downloadProgress); if (!downloadProgress.isCanceled() && !downloadProgress.isError()) { installProgress.startTask("Installing...", ProgressMonitor.UNKNOWN); diff --git a/app/src/processing/app/contrib/ContributionManagerDialog.java b/app/src/processing/app/contrib/ContributionManagerDialog.java index 5606d616c..5bef465ee 100644 --- a/app/src/processing/app/contrib/ContributionManagerDialog.java +++ b/app/src/processing/app/contrib/ContributionManagerDialog.java @@ -80,22 +80,24 @@ public class ContributionManagerDialog { dialog = new JFrame(title); Toolkit.setIcon(dialog); - createComponents(); - registerDisposeListeners(); dialog.pack(); - Dimension screen = Toolkit.getScreenSize(); - dialog.setLocation((screen.width - dialog.getWidth()) / 2, - (screen.height - dialog.getHeight()) / 2); + dialog.setLocationRelativeTo(null); +// Dimension screen = Toolkit.getScreenSize(); +// dialog.setLocation((screen.width - dialog.getWidth()) / 2, +// (screen.height - dialog.getHeight()) / 2); contributionListPanel.grabFocus(); } dialog.setVisible(true); - if (!contribListing.hasDownloadedLatestList()) { + if (contribListing.hasDownloadedLatestList()) { + updateContributionListing(); + + } else { contribListing.downloadAvailableList(new AbstractProgressMonitor() { public void startTask(String name, int maxValue) { } @@ -107,14 +109,13 @@ public class ContributionManagerDialog { updateCategoryChooser(); if (isError()) { status.setErrorMessage("An error occured when downloading " + - "the list of available contributions."); + "the list of available contributions."); } else { status.updateUI(); } } }); } - updateContributionListing(); } diff --git a/app/src/processing/app/contrib/ContributionPanel.java b/app/src/processing/app/contrib/ContributionPanel.java index 2ac7094b7..034de7cd8 100644 --- a/app/src/processing/app/contrib/ContributionPanel.java +++ b/app/src/processing/app/contrib/ContributionPanel.java @@ -59,6 +59,7 @@ class ContributionPanel extends JPanel { private HyperlinkListener conditionalHyperlinkOpener; private JTextPane headerText; private JTextPane descriptionText; + private JTextPane textBlock; private JTextPane updateNotificationLabel; private JButton updateButton; private JProgressBar installProgressBar; @@ -406,6 +407,7 @@ class ContributionPanel extends JPanel { } else if (contrib.isInstalled()) { installRemoveButton.addActionListener(removeActionListener); installRemoveButton.setText("Remove"); + installRemoveButton.setVisible(true); } else { installRemoveButton.addActionListener(installActionListener); installRemoveButton.setText("Install"); @@ -504,7 +506,7 @@ class ContributionPanel extends JPanel { if (contrib != null && !contrib.requiresRestart()) { updateButton.setVisible(isSelected() && contribListing.hasUpdates(contrib)); } - installRemoveButton.setVisible(isSelected()); + installRemoveButton.setVisible(isSelected() || installRemoveButton.getText().equals("Remove")); // for (JTextPane textPane : headerPaneSet) { { @@ -529,7 +531,7 @@ class ContributionPanel extends JPanel { public boolean isSelected() { - return listPanel.selectedPanel == this; + return listPanel.getSelectedPanel() == this; } @@ -621,7 +623,8 @@ class ContributionPanel extends JPanel { HTMLDocument html = (HTMLDocument) doc; StyleSheet stylesheet = html.getStyleSheet(); - String c = installed ? "#808080" : "#000000"; + String c = installed ? "#404040" : "#000000"; // slightly grayed when installed +// String c = "#000000"; // just make them both black stylesheet.addRule("body { color:" + c + "; }"); stylesheet.addRule("a { color:" + c + "; }"); } diff --git a/app/src/processing/app/contrib/ProgressMonitor.java b/app/src/processing/app/contrib/ProgressMonitor.java index b621e6121..616ebfca4 100644 --- a/app/src/processing/app/contrib/ProgressMonitor.java +++ b/app/src/processing/app/contrib/ProgressMonitor.java @@ -174,5 +174,4 @@ abstract class JProgressMonitor extends AbstractProgressMonitor { } public abstract void finishedAction(); - } diff --git a/todo.txt b/todo.txt index db1bff391..88c7b3181 100644 --- a/todo.txt +++ b/todo.txt @@ -129,6 +129,7 @@ X https://github.com/processing/processing/issues/1250 X http://code.google.com/p/processing/issues/detail?id=1212 medium +_ highlight color seems to be incorrect? _ Contributed modes should show up in mode menu after installation _ https://github.com/processing/processing/issues/1504 X http://code.google.com/p/processing/issues/detail?id=1466 @@ -141,9 +142,12 @@ _ or if a bad document comes through, it can recover _ gracefully recover from proxy problems _ https://github.com/processing/processing/issues/1601 _ alternating blue/white backgrounds aren't updated after changing filter +_ just need to call a repaint() after selection change? _ move styling to separate constants that are more accessible _ work on the design of the list entries themselves _ check with Casey about coloring for error messages +_ test on Windows and Linux +_ add "Installed" indicator? or show the Remove button? low _ after installing, the item in the manager list doesn't change color