Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IEP-1255: Adding option to remove all versions #996

Merged
merged 1 commit into from
Jun 21, 2024
Merged

IEP-1255: Adding option to remove all versions #996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .../com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@
{
reload = true;
getIdfToolSets(false);
// cleanup the env and toolchains
IDFEnvironmentVariables idfEnvironmentVariables = new IDFEnvironmentVariables();
idfEnvironmentVariables.removeAllEnvVariables();
try
{
ESPToolChainManager espToolChainManager = new ESPToolChainManager();
espToolChainManager.removeCmakeToolChains();
espToolChainManager.removeStdToolChains();
}
catch (Exception e)
{
Logger.log(e);
}
Comment on lines +53 to +65
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhanced cleanup logic in the delete method ensures that environment variables and toolchains are properly removed. Consider adding more detailed logging to improve traceability.

// Suggested logging enhancement
Logger.log("Starting cleanup for toolset: " + idfToolSet.getId());
try
{
    ESPToolChainManager espToolChainManager = new ESPToolChainManager();
    espToolChainManager.removeCmakeToolChains();
    espToolChainManager.removeStdToolChains();    
}
catch (Exception e)
{
    Logger.log("Failed to clean up toolchains for: " + idfToolSet.getId(), e);
}
Logger.log("Completed cleanup for toolset: " + idfToolSet.getId());


// update the json now to remove the idf from it.
List<IDFToolSet> idfToolSetsToExport = new ArrayList<IDFToolSet>();
for (IDFToolSet idfTool : idfToolSets)
{
Expand All @@ -61,7 +76,7 @@
idfToolSetsToExport.add(idfTool);
}

try (FileWriter fileWriter = new FileWriter(toolSetConfigFilePath()))

Check warning on line 79 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.tools.ToolSetConfigurationManager.delete(IDFToolSet): new java.io.FileWriter(String)
Raw output 
Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
{
gson.toJson(idfToolSetsToExport, fileWriter);
}
Expand Down Expand Up @@ -101,7 +116,7 @@
Logger.log(e);
}
}
return idfToolSets;

Check warning on line 119 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP 

com.espressif.idf.core.tools.ToolSetConfigurationManager.getIdfToolSets(boolean) may expose internal representation by returning ToolSetConfigurationManager.idfToolSets
Raw output 
Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.
}

private List<IDFToolSet> importToolSets()
Expand All @@ -123,7 +138,7 @@
}
}

try (FileReader fileReader = new FileReader(toolSetConfigFilePath()))

Check warning on line 141 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.tools.ToolSetConfigurationManager.importToolSets(): new java.io.FileReader(String)
Raw output 
Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
{
idfToolSets = gson.fromJson(fileReader, listType);
}
Expand Down Expand Up @@ -167,7 +182,7 @@

if (toolSetConfigFile.exists())
{
try (FileReader fileReader = new FileReader(toolSetConfigFile))

Check warning on line 185 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.tools.ToolSetConfigurationManager.export(IDFToolSet): new java.io.FileReader(File)
Raw output 
Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
{
idfToolSets = gson.fromJson(fileReader, listType);
}
Expand Down Expand Up @@ -209,7 +224,7 @@
idfToolSets.add(idfToolSet);
}

try (FileWriter fileWriter = new FileWriter(toolSetConfigFile))

Check warning on line 227 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.tools.ToolSetConfigurationManager.export(IDFToolSet): new java.io.FileWriter(File)
Raw output 
Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
{
gson.toJson(idfToolSets, fileWriter);
}
Expand All @@ -224,7 +239,7 @@
{
reload = true;
getIdfToolSets(false);
List<IDFToolSet> idfToolSetsToExport = new ArrayList<IDFToolSet>();

Check warning on line 242 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

UC_USELESS_OBJECT 

Useless object stored in variable idfToolSetsToExport of method com.espressif.idf.core.tools.ToolSetConfigurationManager.updateToolSetConfiguration(IDFToolSet)
Raw output 
Our analysis shows that this object is useless. It's created and modified, but its value never go outside of the method or produce any side-effect. Either there is a mistake and object was intended to be used or it can be removed.

This analysis rarely produces false-positives. Common false-positive cases include:

- This object used to implicitly throw some obscure exception.

- This object used as a stub to generalize the code.

- This object used to hold strong references to weak/soft-referenced objects.
for (IDFToolSet existingIdfToolSet : idfToolSets)
{
if (idfToolSet.getId() == existingIdfToolSet.getId())
Expand All @@ -237,7 +252,7 @@
}
}

try (FileWriter fileWriter = new FileWriter(toolSetConfigFilePath()))

Check warning on line 255 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolSetConfigurationManager.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.tools.ToolSetConfigurationManager.updateToolSetConfiguration(IDFToolSet): new java.io.FileWriter(String)
Raw output 
Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.
{
gson.toJson(idfToolSets, fileWriter);
}
Expand Down
2 changes: 1 addition & 1 deletion bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFNewToolsWizard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@

public IDFNewToolsWizard(ESPIDFMainTablePage espidfMainTablePage)
{
this.espidfMainTablePage = espidfMainTablePage;

Check warning on line 61 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFNewToolsWizard.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP2 

new com.espressif.idf.ui.install.IDFNewToolsWizard(ESPIDFMainTablePage) may expose internal representation by storing an externally mutable object into IDFNewToolsWizard.espidfMainTablePage
Raw output 
This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
toolSetConfigurationManager = new ToolSetConfigurationManager();
}

Expand All @@ -82,7 +82,7 @@
}
else
{
new File(destinationLocation).mkdirs();

Check warning on line 85 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFNewToolsWizard.java

View workflow job for this annotation

GitHub Actions / spotbugs

RV_RETURN_VALUE_IGNORED_BAD_PRACTICE 

Exceptional return value of java.io.File.mkdirs() ignored in com.espressif.idf.ui.install.IDFNewToolsWizard.performFinish()
Raw output 
This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.
String url = version.getUrl();
if (version.getName().equals("master") || version.getName().startsWith("release/")) //$NON-NLS-1$ //$NON-NLS-2$
{
Expand Down Expand Up @@ -151,7 +151,7 @@
if (downloadFile != null)
{
unZipFile(downloadFile, destinationLocation);
new File(downloadFile).delete();

Check warning on line 154 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFNewToolsWizard.java

View workflow job for this annotation

GitHub Actions / spotbugs

RV_RETURN_VALUE_IGNORED_BAD_PRACTICE 

Exceptional return value of java.io.File.delete() ignored in com.espressif.idf.ui.install.IDFNewToolsWizard.download(IProgressMonitor, String, String)
Raw output 
This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.

// extracts file name from URL
String folderName = new File(url).getName().replace(".zip", ""); //$NON-NLS-1$ //$NON-NLS-2$
Expand Down Expand Up @@ -277,7 +277,7 @@
String saveFilePath = saveDir + File.separator + fileName;

// opens an output stream to save into file
FileOutputStream outputStream = new FileOutputStream(saveFilePath);

Check warning on line 280 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFNewToolsWizard.java

View workflow job for this annotation

GitHub Actions / spotbugs

OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE 

com.espressif.idf.ui.install.IDFNewToolsWizard.downloadFile(String, String, IProgressMonitor) may fail to clean up java.io.OutputStream on checked exception
Raw output 
This method may fail to clean up (close, dispose of) a stream, database object, or other resource requiring an explicit cleanup operation.

In general, if a method opens a stream or other resource, the method should use a try/finally block to ensure that the stream or resource is cleaned up before the method returns.

This bug pattern is essentially the same as the OS_OPEN_STREAM and ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and hopefully better) static analysis technique. We are interested is getting feedback about the usefulness of this bug pattern. For sending feedback, check:

 * contributing guideline [https://github.com/spotbugs/spotbugs/blob/master/.github/CONTRIBUTING.md]
 * mailinglist [https://github.com/spotbugs/discuss/issues?q=]

In particular, the false-positive suppression heuristics for this bug pattern have not been extensively tuned, so reports about false positives are helpful to us.

See Weimer and Necula, Finding and Preventing Run-Time Error Handling Mistakes (PDF [https://people.eecs.berkeley.edu/~necula/Papers/rte_oopsla04.pdf]), for a description of the analysis technique.

float downloaded = 0f;
int bytesRead = -1;
Expand Down Expand Up @@ -333,7 +333,7 @@
Display.getDefault().asyncExec(() -> {
if (espidfMainTablePage != null)
{
espidfMainTablePage.refreshTable();
espidfMainTablePage.refreshEditorUI();
}
});

Expand Down
5 changes: 3 additions & 2 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,12 @@ public class Messages extends NLS
public static String EspIdfManagerStateCol;
public static String EspIdfManagerActivateCol;
public static String EspIdfManagerAddToolsBtn;
public static String EspIdfManagerRemoveAllBtn;
public static String EspIdfManagerDeleteBtn;
public static String EspIdfManagerMessageBoxActiveToolchainDelete;
public static String EspIdfManagerMessageBoxActiveToolchainDeleteTitle;
public static String EspIdfManagerMessageBoxDeleteConfirmMessage;
public static String EspIdfManagerMessageBoxDeleteConfirmMessageTitle;
public static String EspIdfManagerMessageBoxDeleteAllConfirmMessage;
public static String EspIdfManagerMessageBoxDeleteAllConfirmMessageTitle;

public static String IDFDownloadHandler_ESPIDFConfiguration;
public static String IDFDownloadHandler_DownloadPage_Title;
Expand Down
2 changes: 1 addition & 1 deletion bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsActivationJobListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

public ToolsActivationJobListener(ESPIDFMainTablePage espidfMainTablePage)
{
this.espidfMainTablePage = espidfMainTablePage;

Check warning on line 32 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsActivationJobListener.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP2 

new com.espressif.idf.ui.tools.ToolsActivationJobListener(ESPIDFMainTablePage) may expose internal representation by storing an externally mutable object into ToolsActivationJobListener.espidfMainTablePage
Raw output 
This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
}

public ToolsActivationJobListener()
Expand All @@ -49,7 +49,7 @@
Display.getDefault().asyncExec(() -> {
if (espidfMainTablePage != null)
{
espidfMainTablePage.refreshTable();
espidfMainTablePage.refreshEditorUI();
}
});
OpenDialogListenerSupport.getSupport().firePropertyChange(PopupDialog.ENABLE_LAUNCHBAR_EVENTS.name(), null,
Expand Down
124 changes: 82 additions & 42 deletions ...om.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package com.espressif.idf.ui.tools.manager.pages;

import java.text.MessageFormat;
import java.util.List;

import org.eclipse.jface.layout.TableColumnLayout;
import org.eclipse.jface.viewers.ArrayContentProvider;
Expand Down Expand Up @@ -35,6 +36,7 @@
import com.espressif.idf.core.tools.ToolSetConfigurationManager;
import com.espressif.idf.core.tools.vo.IDFToolSet;
import com.espressif.idf.core.util.IDFUtil;
import com.espressif.idf.ui.LaunchBarListener;
import com.espressif.idf.ui.UIPlugin;
import com.espressif.idf.ui.install.IDFNewToolsWizard;
import com.espressif.idf.ui.tools.Messages;
Expand All @@ -58,23 +60,26 @@
private TableViewerColumn removeColumn;
private TableColumnLayout tableColumnLayout;
private Composite tableComposite;

private List<IDFToolSet> idfToolSets;
private Button removeAllButton;

private static final String REMOVE_ICON = "icons/tools/delete.png"; //$NON-NLS-1$
private static final String RELOAD_ICON = "icons/tools/reload.png"; //$NON-NLS-1$
private static final String IDF_TOOL_SET_BTN_KEY = "IDFToolSet"; //$NON-NLS-1$

public Composite createPage(Composite composite)
{
toolSetConfigurationManager = new ToolSetConfigurationManager();
idfToolSets = toolSetConfigurationManager.getIdfToolSets(true);
container = new Composite(composite, SWT.NONE);
final int numColumns = 2;
GridLayout gridLayout = new GridLayout(numColumns, false);
container.setLayout(gridLayout);
createIdfTable(container);
return container;

Check warning on line 79 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP 

com.espressif.idf.ui.tools.manager.pages.ESPIDFMainTablePage.createPage(Composite) may expose internal representation by returning ESPIDFMainTablePage.container
Raw output 
Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.
}

public void refreshTable()
public void refreshEditorUI()
{
for (TableItem item : tableViewer.getTable().getItems())
{
Expand Down Expand Up @@ -104,12 +109,22 @@
}
}
toolSetConfigurationManager.setReload(true);
idfToolSets = toolSetConfigurationManager.getIdfToolSets(true);
setupColumns();
tableViewer.setInput(toolSetConfigurationManager.getIdfToolSets(true));
tableViewer.setInput(idfToolSets);
tableViewer.getControl().requestLayout();
tableViewer.refresh();
container.redraw();
toolSetConfigurationManager.setReload(false);

if (idfToolSets == null || idfToolSets.isEmpty())
{
removeAllButton.setEnabled(false);
}
else
{
removeAllButton.setEnabled(true);
}
}

private Composite createIdfTable(Composite parent)
Expand Down Expand Up @@ -144,15 +159,19 @@
GridData buttonCompositeGridData = new GridData(SWT.RIGHT, SWT.CENTER, false, false);
buttonCompositeGridData.verticalAlignment = SWT.TOP; // Aligns the button composite at the top
buttonComposite.setLayoutData(buttonCompositeGridData);
buttonComposite.setLayout(new FillLayout());
buttonComposite.setLayout(new GridLayout(1, true));

// Creating the "Add" button
Button addButton = new Button(buttonComposite, SWT.PUSH);
addButton.setText(Messages.EspIdfManagerAddToolsBtn);
addButton.addSelectionListener(new SelectionAdapter() {
@Override
public void widgetSelected(SelectionEvent e) {
IDFNewToolsWizard wizard = new IDFNewToolsWizard(ESPIDFMainTablePage.this);
GridData addButtonGridData = new GridData(SWT.FILL, SWT.CENTER, true, false);
addButton.setLayoutData(addButtonGridData);
addButton.addSelectionListener(new SelectionAdapter()
{
@Override
public void widgetSelected(SelectionEvent e)
{
IDFNewToolsWizard wizard = new IDFNewToolsWizard(ESPIDFMainTablePage.this);
wizard.setWindowTitle(Messages.IDFDownloadHandler_ESPIDFConfiguration);
WizardDialog wizDialog = new WizardDialog(container.getShell(), wizard);
wizDialog.create();
Expand All @@ -162,9 +181,43 @@
wizDialog.getShell().setSize(Math.max(850, wizDialog.getShell().getSize().x), 550);

wizDialog.open();
}
});
}
});

removeAllButton = new Button(buttonComposite, SWT.PUSH);
removeAllButton.setText(Messages.EspIdfManagerRemoveAllBtn);
removeAllButton.addSelectionListener(new SelectionAdapter()
{
@Override
public void widgetSelected(SelectionEvent e)
{
MessageBox messageBox = new MessageBox(Display.getDefault().getActiveShell(),
SWT.ICON_WARNING | SWT.YES | SWT.NO);
messageBox.setMessage(Messages.EspIdfManagerMessageBoxDeleteAllConfirmMessage);
messageBox.setText(Messages.EspIdfManagerMessageBoxDeleteAllConfirmMessageTitle);
int response = messageBox.open();
if (response == SWT.YES)
{
for(IDFToolSet idfToolSet : idfToolSets)
{
toolSetConfigurationManager.delete(idfToolSet);
}
refreshEditorUI();
}
}
});
Comment on lines +187 to +208
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added functionality for the removeAllButton to handle the removal of all toolsets. The use of a confirmation dialog box is a good user experience practice. However, consider handling potential exceptions during the deletion process to improve robustness.

+ try {
+   for(IDFToolSet idfToolSet : idfToolSets) {
+     toolSetConfigurationManager.delete(idfToolSet);
+   }
+   refreshEditorUI();
+ } catch (Exception e) {
+   // Log error or inform user
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
removeAllButton = new Button(buttonComposite, SWT.PUSH);
removeAllButton.setText(Messages.EspIdfManagerRemoveAllBtn);
removeAllButton.addSelectionListener(new SelectionAdapter()
{
@Override
public void widgetSelected(SelectionEvent e)
{
MessageBox messageBox = new MessageBox(Display.getDefault().getActiveShell(),
SWT.ICON_WARNING | SWT.YES | SWT.NO);
messageBox.setMessage(Messages.EspIdfManagerMessageBoxDeleteAllConfirmMessage);
messageBox.setText(Messages.EspIdfManagerMessageBoxDeleteAllConfirmMessageTitle);
int response = messageBox.open();
if (response == SWT.YES)
{
for(IDFToolSet idfToolSet : idfToolSets)
{
toolSetConfigurationManager.delete(idfToolSet);
}
refreshEditorUI();
}
}
});
removeAllButton = new Button(buttonComposite, SWT.PUSH);
removeAllButton.setText(Messages.EspIdfManagerRemoveAllBtn);
removeAllButton.addSelectionListener(new SelectionAdapter()
{
@Override
public void widgetSelected(SelectionEvent e)
{
MessageBox messageBox = new MessageBox(Display.getDefault().getActiveShell(),
SWT.ICON_WARNING | SWT.YES | SWT.NO);
messageBox.setMessage(Messages.EspIdfManagerMessageBoxDeleteAllConfirmMessage);
messageBox.setText(Messages.EspIdfManagerMessageBoxDeleteAllConfirmMessageTitle);
int response = messageBox.open();
if (response == SWT.YES)
{
try {
for(IDFToolSet idfToolSet : idfToolSets) {
toolSetConfigurationManager.delete(idfToolSet);
}
refreshEditorUI();
} catch (Exception e) {
// Log error or inform user
}
}
}
});


GridData removeAllButtonGridData = new GridData(SWT.FILL, SWT.CENTER, true, false);
removeAllButton.setLayoutData(removeAllButtonGridData);
if (idfToolSets == null || idfToolSets.isEmpty())
{
removeAllButton.setEnabled(false);
}
else
{
removeAllButton.setEnabled(true);
}

return idfToolsGroup;
}

Expand Down Expand Up @@ -246,25 +299,14 @@

private void performDeleteOperation(IDFToolSet idfToolSet)
{
if (idfToolSet.isActive())
MessageBox messageBox = new MessageBox(Display.getDefault().getActiveShell(),
SWT.ICON_WARNING | SWT.YES | SWT.NO);
messageBox.setMessage(MessageFormat.format(Messages.EspIdfManagerMessageBoxDeleteConfirmMessage, idfToolSet.getIdfVersion()));
messageBox.setText(Messages.EspIdfManagerMessageBoxDeleteConfirmMessageTitle);
int response = messageBox.open();
if (response == SWT.YES)
{
MessageBox messageBox = new MessageBox(Display.getDefault().getActiveShell(),
SWT.ICON_INFORMATION | SWT.OK);
messageBox.setMessage(Messages.EspIdfManagerMessageBoxActiveToolchainDelete);
messageBox.setText(Messages.EspIdfManagerMessageBoxActiveToolchainDeleteTitle);
messageBox.open();
}
else
{
MessageBox messageBox = new MessageBox(Display.getDefault().getActiveShell(),
SWT.ICON_WARNING | SWT.YES | SWT.NO);
messageBox.setMessage(MessageFormat.format(Messages.EspIdfManagerMessageBoxDeleteConfirmMessage, idfToolSet.getIdfVersion()));
messageBox.setText(Messages.EspIdfManagerMessageBoxDeleteConfirmMessageTitle);
int response = messageBox.open();
if (response == SWT.YES)
{
toolSetConfigurationManager.delete(idfToolSet);
}
toolSetConfigurationManager.delete(idfToolSet);
}
}

Expand Down Expand Up @@ -409,20 +451,18 @@
toolsActivationJob.schedule();
});
}
else
{
Button removeButton = new Button(buttonComposite, SWT.PUSH | SWT.FLAT);
removeButton.pack();
removeButton.setData(IDF_TOOL_SET_BTN_KEY, idfToolSet);
removeButton.setImage(UIPlugin.getImage(REMOVE_ICON));
removeButton.setToolTipText(Messages.EspIdfManagerDeleteBtnToolTip);
removeButton.addListener(SWT.Selection, e -> {
Button btn = (Button) e.widget;
IDFToolSet selectedToolSet = (IDFToolSet) btn.getData(IDF_TOOL_SET_BTN_KEY);
performDeleteOperation(selectedToolSet);
refreshTable();
});
}

Button removeButton = new Button(buttonComposite, SWT.PUSH | SWT.FLAT);
removeButton.pack();
removeButton.setData(IDF_TOOL_SET_BTN_KEY, idfToolSet);
removeButton.setImage(UIPlugin.getImage(REMOVE_ICON));
removeButton.setToolTipText(Messages.EspIdfManagerDeleteBtnToolTip);
removeButton.addListener(SWT.Selection, e -> {
Button btn = (Button) e.widget;
IDFToolSet selectedToolSet = (IDFToolSet) btn.getData(IDF_TOOL_SET_BTN_KEY);
performDeleteOperation(selectedToolSet);
refreshEditorUI();
});

editor.grabHorizontal = true;
editor.setEditor(buttonComposite, item, cell.getColumnIndex());
Expand All @@ -447,56 +487,56 @@
{
private int propertyIndex;
private static final int DESCENDING = 1;
private int direction = 0;

public ColumnViewerComparator()
{
this.propertyIndex = 0;
direction = DESCENDING;
}

public void setColumn(int column)
{
if (column == this.propertyIndex)
{
// If the same column is clicked again, toggle the direction
direction = 1 - direction;
}
else
{
// Else, sort the new column in ascending order
this.propertyIndex = column;
direction = DESCENDING;
}
}

@Override
public int compare(Viewer viewer, Object e1, Object e2)
{
IDFToolSet p1 = (IDFToolSet) e1;
IDFToolSet p2 = (IDFToolSet) e2;
int rc = 0;
switch (propertyIndex)
{
case 0:
rc = p1.getIdfVersion().compareTo(p2.getIdfVersion());
break;
case 1:
rc = p1.getIdfLocation().compareTo(p2.getIdfLocation());
break;
case 2:
Boolean p1State = p1.isActive();
Boolean p2State = p2.isActive();
rc = p1State.compareTo(p2State);
break;
default:
break;
}
if (direction == DESCENDING)
{
rc = -rc;
}
return rc;

Check warning on line 539 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java

View workflow job for this annotation

GitHub Actions / spotbugs

SIC_INNER_SHOULD_BE_STATIC 

Should com.espressif.idf.ui.tools.manager.pages.ESPIDFMainTablePage$ColumnViewerComparator be a _static_ inner class?
Raw output 
This class is an inner class, but does not use its embedded reference to the object which created it.  This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.  If possible, the class should be made static.
}

}
Expand Down
9 changes: 5 additions & 4 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,12 @@ EspIdfManagerLocationCol=Location
EspIdfManagerStateCol=State
EspIdfManagerActivateCol=Active
EspIdfManagerAddToolsBtn=Add ESP-IDF
EspIdfManagerRemoveAllBtn=Remove All
EspIdfManagerDeleteBtn=Delete
EspIdfManagerMessageBoxActiveToolchainDelete=You are trying to delete active toolchain. Please switch active toolchain to delete.
EspIdfManagerMessageBoxActiveToolchainDeleteTitle=Cannot delete active toolchain
EspIdfManagerMessageBoxDeleteConfirmMessage=Do you want to delete the toolchain with ESP-IDF version: {0}
EspIdfManagerMessageBoxDeleteConfirmMessageTitle=Confirm Delete
EspIdfManagerMessageBoxDeleteConfirmMessage=Do you want to remove the toolchain with ESP-IDF version: {0}
EspIdfManagerMessageBoxDeleteConfirmMessageTitle=Confirm Remove
EspIdfManagerMessageBoxDeleteAllConfirmMessage=Do you want to remove all ESP-IDF versions from IDE
EspIdfManagerMessageBoxDeleteAllConfirmMessageTitle=Remove All ESP-IDF Versions
EspIdfManagerReloadBtnToolTip=Reload from disk
EspIdfManagerDeleteBtnToolTip=Delete ESP-IDF version from IDE

Expand Down
Loading