From 3b210c1c311be7b7d6cb8b19505e1359ddd406fd Mon Sep 17 00:00:00 2001 From: Ben Fry Date: Sat, 14 Jan 2023 14:01:57 -0500 Subject: [PATCH] cleaning up some naming and the order of functions for clarity --- .../app/contrib/ContributionListing.java | 101 ++++++++++-------- .../app/contrib/ContributionManager.java | 4 +- .../app/contrib/ContributionTab.java | 11 +- app/src/processing/app/contrib/ListPanel.java | 6 +- .../app/contrib/LocalContribution.java | 2 +- .../processing/app/contrib/StatusDetail.java | 5 +- 6 files changed, 70 insertions(+), 59 deletions(-) diff --git a/app/src/processing/app/contrib/ContributionListing.java b/app/src/processing/app/contrib/ContributionListing.java index 419e42059..d00bbb833 100644 --- a/app/src/processing/app/contrib/ContributionListing.java +++ b/app/src/processing/app/contrib/ContributionListing.java @@ -49,28 +49,29 @@ public class ContributionListing { /** Location of the listing file on disk, will be read and written. */ File listingFile; - - Set listPanels; - final List advertisedContributions; - Map librariesByImportHeader; - Set allContributions; boolean listDownloaded; // boolean listDownloadFailed; ReentrantLock downloadingLock; + final Set availableContribs; + Map librariesByImportHeader; + Set allContribs; + + Set listPanels; + private ContributionListing() { listPanels = new HashSet<>(); - advertisedContributions = new ArrayList<>(); + availableContribs = new HashSet<>(); librariesByImportHeader = new HashMap<>(); - allContributions = ConcurrentHashMap.newKeySet(); + allContribs = ConcurrentHashMap.newKeySet(); downloadingLock = new ReentrantLock(); listingFile = Base.getSettingsFile(LOCAL_FILENAME); if (listingFile.exists()) { // On the EDT already, but do this later on the EDT so that the // constructor can finish more efficiently inside getInstance(). - EventQueue.invokeLater(() -> setAdvertisedList(listingFile)); + EventQueue.invokeLater(() -> loadAvailableList(listingFile)); } } @@ -87,20 +88,10 @@ public class ContributionListing { } - private void setAdvertisedList(File file) { - listingFile = file; - - advertisedContributions.clear(); - advertisedContributions.addAll(parseContribList(listingFile)); - for (Contribution contribution : advertisedContributions) { - addContribution(contribution); - } - } - - /** - * Adds the installed libraries to the listing of libraries, replacing - * any pre-existing libraries by the same name as one in the list. + * Update the list of contribs with entries for what is installed. + * If it matches an entry from contribs.txt, replace that entry. + * If not, add it to the list as a new contrib. */ protected void updateInstalled(Set installed) { // Map replacements = new HashMap<>(); @@ -130,7 +121,7 @@ public class ContributionListing { private Contribution findContribution(Contribution contribution) { - for (Contribution c : allContributions) { + for (Contribution c : allContribs) { if (c.getName().equals(contribution.getName()) && c.getType() == contribution.getType()) { return c; @@ -149,8 +140,8 @@ public class ContributionListing { } } } - allContributions.remove(oldContrib); - allContributions.add(newContrib); + allContribs.remove(oldContrib); + allContribs.add(newContrib); for (ListPanel listener : listPanels) { listener.contributionChanged(oldContrib, newContrib); @@ -165,7 +156,7 @@ public class ContributionListing { getLibrariesByImportHeader().put(importName, contribution); } } - allContributions.add(contribution); + allContribs.add(contribution); for (ListPanel listener : listPanels) { listener.contributionAdded(contribution); @@ -179,7 +170,7 @@ public class ContributionListing { getLibrariesByImportHeader().remove(importName); } } - allContributions.remove(contribution); + allContribs.remove(contribution); for (ListPanel listener : listPanels) { listener.contributionRemoved(contribution); @@ -187,11 +178,15 @@ public class ContributionListing { } - protected AvailableContribution getAvailableContribution(Contribution info) { - synchronized (advertisedContributions) { - for (AvailableContribution advertised : advertisedContributions) { - if (advertised.getType() == info.getType() && - advertised.getName().equals(info.getName())) { + /** + * Given a contribution that's already installed, find it in the list + * of available contributions to see if there is an update available. + */ + protected AvailableContribution findAvailableContribution(Contribution contrib) { + synchronized (availableContribs) { + for (AvailableContribution advertised : availableContribs) { + if (advertised.getType() == contrib.getType() && + advertised.getName().equals(contrib.getName())) { return advertised; } } @@ -229,6 +224,7 @@ public class ContributionListing { tempContribFile, progress); if (progress.notCanceled() && !progress.isException()) { if (listingFile.exists()) { + //noinspection ResultOfMethodCallIgnored listingFile.delete(); // may silently fail, but below may still work } if (tempContribFile.renameTo(listingFile)) { @@ -237,7 +233,7 @@ public class ContributionListing { try { // TODO: run this in SwingWorker done() [jv] EventQueue.invokeAndWait(() -> { - setAdvertisedList(listingFile); + loadAvailableList(listingFile); base.tallyUpdatesAvailable(); }); } catch (InterruptedException e) { @@ -265,6 +261,22 @@ public class ContributionListing { } + protected boolean isDownloaded() { + return listDownloaded; + } + + + private void loadAvailableList(File file) { + listingFile = file; + + availableContribs.clear(); + availableContribs.addAll(parseContribList(listingFile)); + for (Contribution contribution : availableContribs) { + addContribution(contribution); + } + } + + /** * Bundles information about what contribs are installed, so that they can * be reported at the stats link. @@ -289,27 +301,24 @@ public class ContributionListing { public boolean hasUpdates(Contribution contrib) { if (contrib.isInstalled()) { - Contribution advertised = getAvailableContribution(contrib); - if (advertised != null) { - return (advertised.getVersion() > contrib.getVersion() && - advertised.isCompatible(Base.getRevision())); - } + Contribution available = findAvailableContribution(contrib); + return available != null && + (available.getVersion() > contrib.getVersion() && + available.isCompatible(Base.getRevision())); } return false; } + /** + * Get the human-readable version number from the available list. + */ protected String getLatestPrettyVersion(Contribution contrib) { - Contribution newestContrib = getAvailableContribution(contrib); - if (newestContrib == null) { - return null; + Contribution newestContrib = findAvailableContribution(contrib); + if (newestContrib != null) { + return newestContrib.getPrettyVersion(); } - return newestContrib.getPrettyVersion(); - } - - - protected boolean isDownloaded() { - return listDownloaded; + return null; } diff --git a/app/src/processing/app/contrib/ContributionManager.java b/app/src/processing/app/contrib/ContributionManager.java index 9257070c4..e635282da 100644 --- a/app/src/processing/app/contrib/ContributionManager.java +++ b/app/src/processing/app/contrib/ContributionManager.java @@ -552,7 +552,7 @@ public class ContributionManager { // https://github.com/processing/processing/issues/5823 if (installList != null) { for (File file : installList) { - for (AvailableContribution contrib : contribListing.advertisedContributions) { + for (AvailableContribution contrib : contribListing.availableContribs) { if (file.getName().equals(contrib.getName())) { file.delete(); installOnStartUp(base, contrib); @@ -638,7 +638,7 @@ public class ContributionManager { } } - for (AvailableContribution contrib : contribListing.advertisedContributions) { + for (AvailableContribution contrib : contribListing.availableContribs) { if (updateContribsNames.contains(contrib.getName())) { updateContribsList.add(contrib); } diff --git a/app/src/processing/app/contrib/ContributionTab.java b/app/src/processing/app/contrib/ContributionTab.java index 9f409dc61..dbbf4e8cb 100644 --- a/app/src/processing/app/contrib/ContributionTab.java +++ b/app/src/processing/app/contrib/ContributionTab.java @@ -277,7 +277,7 @@ public class ContributionTab extends JPanel { private Set listCategories() { Set categories = new HashSet<>(); - for (Contribution c : ContributionListing.getInstance().allContributions) { + for (Contribution c : ContributionListing.getInstance().allContribs) { if (filter.matches(c)) { for (String category : c.getCategories()) { categories.add(category); @@ -293,8 +293,11 @@ public class ContributionTab extends JPanel { categoryChooser.removeAllItems(); Set categories = listCategories(); - if (categories.size() == 1 && - categories.contains(Contribution.UNKNOWN_CATEGORY)) { + // this is a complicated way of saying "if this is the libraries tab" + //if (categories.size() == 1 && + // categories.contains(Contribution.UNKNOWN_CATEGORY)) { + if (categories.isEmpty() || // listing not loaded yet + contribType != ContributionType.LIBRARY) { // Add dummy item for sizing purposes // https://github.com/processing/processing4/issues/520 categoryChooser.addItem("NULL"); @@ -329,12 +332,12 @@ public class ContributionTab extends JPanel { } - /* // TODO Why is this entire set of code only running when Editor // is not null... And what's it doing anyway? Shouldn't it run // on all editors? (The change to getActiveEditor() was made // for 4.0b8 because the Editor may have been closed after the // Contrib Manager was opened.) [fry 220311] + /* protected void updateContributionListing() { Editor editor = base.getActiveEditor(); if (editor != null) { diff --git a/app/src/processing/app/contrib/ListPanel.java b/app/src/processing/app/contrib/ListPanel.java index e7dd0fbc8..1d365c62c 100644 --- a/app/src/processing/app/contrib/ListPanel.java +++ b/app/src/processing/app/contrib/ListPanel.java @@ -515,7 +515,7 @@ public class ListPanel extends JPanel implements Scrollable { @Override public int getRowCount() { - return ContributionListing.getInstance().allContributions.size() + (sectionsEnabled ? 4 : 0); + return ContributionListing.getInstance().allContribs.size() + (sectionsEnabled ? 4 : 0); } @Override @@ -539,7 +539,7 @@ public class ListPanel extends JPanel implements Scrollable { @Override public Object getValueAt(int rowIndex, int columnIndex) { final Set allContribs = - ContributionListing.getInstance().allContributions; + ContributionListing.getInstance().allContribs; if (rowIndex >= allContribs.size()) { return sections[rowIndex - allContribs.size()]; } @@ -603,7 +603,7 @@ public class ListPanel extends JPanel implements Scrollable { } private boolean includeSection(SectionHeaderContribution section) { - return ContributionListing.getInstance().allContributions.stream() + return ContributionListing.getInstance().allContribs.stream() .filter(contribution -> contribution.getType() == section.getType()) .anyMatch(this::includeContribution); } diff --git a/app/src/processing/app/contrib/LocalContribution.java b/app/src/processing/app/contrib/LocalContribution.java index 216f6b61c..38bcfd1af 100644 --- a/app/src/processing/app/contrib/LocalContribution.java +++ b/app/src/processing/app/contrib/LocalContribution.java @@ -394,7 +394,7 @@ public abstract class LocalContribution extends Contribution { ContributionListing cl = ContributionListing.getInstance(); Contribution advertisedVersion = - cl.getAvailableContribution(LocalContribution.this); + cl.findAvailableContribution(LocalContribution.this); if (advertisedVersion == null) { cl.removeContribution(LocalContribution.this); diff --git a/app/src/processing/app/contrib/StatusDetail.java b/app/src/processing/app/contrib/StatusDetail.java index 1566be959..11298975e 100644 --- a/app/src/processing/app/contrib/StatusDetail.java +++ b/app/src/processing/app/contrib/StatusDetail.java @@ -30,7 +30,6 @@ import javax.swing.JProgressBar; import processing.app.*; import processing.app.laf.PdeProgressBarUI; -import processing.app.ui.Toolkit; /** @@ -179,7 +178,7 @@ class StatusDetail { public void finishedAction() { statusPanel.resetProgressBar(); AvailableContribution ad = - contribListing.getAvailableContribution(contrib); + contribListing.findAvailableContribution(contrib); // install the new version of the Mode (or Tool) installContribution(ad, ad.link); } @@ -201,7 +200,7 @@ class StatusDetail { } else { AvailableContribution ad = - contribListing.getAvailableContribution(contrib); + contribListing.findAvailableContribution(contrib); installContribution(ad, ad.link); } }