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

Jump to bottom

IEP-1049: Add a validation for esp-idf path in the install tools dialog #986

Merged
merged 8 commits into from
Aug 30, 2024
Merged

IEP-1049: Add a validation for esp-idf path in the install tools dialog #986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0c001df
updated message and tools validation added
alirana01 Jun 12, 2024
15f9f18
review comments updated
alirana01 Jul 3, 2024
7697ec4
review comment to improve check
alirana01 Jul 3, 2024
7dc27fd
updated to handle the error message on wizard also changes to ensure …
alirana01 Jul 15, 2024
5ed5268
validation updated for download
alirana01 Jul 23, 2024
79a6e0e
imports cleanup
alirana01 Jul 24, 2024
17e960d
fix for the empty folder issue
alirana01 Aug 14, 2024
4997418
review comments resolved
alirana01 Aug 29, 2024
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
1 change: 1 addition & 0 deletions bundles/com.espressif.idf.swt.custom/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/bin/
157 changes: 128 additions & 29 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,19 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.Platform;
import org.eclipse.jface.dialogs.IDialogConstants;
import org.eclipse.jface.dialogs.MessageDialog;
import org.eclipse.jface.wizard.WizardPage;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.ModifyEvent;
Expand All @@ -29,7 +33,6 @@
import org.eclipse.swt.widgets.Combo;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.DirectoryDialog;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.FileDialog;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
Expand All @@ -38,9 +41,13 @@
import org.eclipse.ui.PartInitException;
import org.eclipse.ui.PlatformUI;

import com.espressif.idf.core.IDFConstants;
import com.espressif.idf.core.IDFCorePlugin;
import com.espressif.idf.core.IDFVersion;
import com.espressif.idf.core.IDFVersionsReader;
import com.espressif.idf.core.ProcessBuilderFactory;
import com.espressif.idf.core.SystemExecutableFinder;
import com.espressif.idf.core.logging.Logger;
import com.espressif.idf.core.util.IDFUtil;
import com.espressif.idf.core.util.StringUtil;
import com.espressif.idf.ui.UIPlugin;
Expand Down Expand Up @@ -105,6 +112,14 @@
GridData gridData = new GridData(SWT.NONE, SWT.NONE, false, false, 2, 1);
gridData.widthHint = 250;
versionCombo.setLayoutData(gridData);
versionCombo.addSelectionListener(new SelectionAdapter()
{
@Override
public void widgetSelected(SelectionEvent e)
{
validate();
}
});

versionsMap = new IDFVersionsReader().getVersionsMap();
Set<String> keySet = versionsMap.keySet();
Expand Down Expand Up @@ -388,18 +403,30 @@
setPageComplete(false);
return;
}
if (idfPath.contains(" ")) //$NON-NLS-1$

String version = getIdfVersionUsingGit(idfPath);

if (idfPath.contains(" ") && !StringUtil.isEmpty(version)) //$NON-NLS-1$
{
setErrorMessage(Messages.IDFDownloadPage_IDFBuildNotSupported);
setPageComplete(false);
showMessage();
return;
boolean validVersion = isVersionGreaterOrEqual(version, "5.0.0"); //$NON-NLS-1$
if (!validVersion)
{
setErrorMessage(Messages.IDFDownloadPage_VersionSpaceError);
setPageComplete(false);
return;
}
if (getErrorMessage() == null || getErrorMessage().equals(Messages.IDFDownloadPage_VersionSpaceError))
{
setErrorMessage(null);
setPageComplete(true);
}
}

String idfPyPath = idfPath + File.separator + "tools" + File.separator + "idf.py"; //$NON-NLS-1$ //$NON-NLS-2$
if (!new File (idfPyPath).exists())
String idfPyPath = idfPath + File.separator + IDFConstants.TOOLS_FOLDER + File.separator + IDFConstants.IDF_PYTHON_SCRIPT;
String idfToolsPyPth = idfPath + File.separator + IDFConstants.TOOLS_FOLDER + File.separator + IDFConstants.IDF_TOOLS_SCRIPT;
if (!new File (idfPyPath).exists() || !new File (idfToolsPyPth).exists())
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use constants for idf.py and idf_tools.py from the IDFConstants file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindIDFpy, idfPath));
setErrorMessage(MessageFormat.format(Messages.IDFDownloadPage_CantfindProperEspIDFDirectory, idfPath));
setPageComplete(false);
return;
}
Expand Down Expand Up @@ -437,7 +464,7 @@

if (!supportSpaces && directoryTxt.getText().contains(" ")) //$NON-NLS-1$
{
setErrorMessage(Messages.IDFDownloadPage_IDFBuildNotSupported);
setErrorMessage(Messages.IDFDownloadPage_VersionSpaceError);
setPageComplete(false);
return;
}
Expand All @@ -455,11 +482,101 @@
}
}

private String getIdfVersionUsingGit(String idfPath)
{
if (!validateGitAndPython())
return StringUtil.EMPTY;

List<String> commands = new ArrayList<String>();
commands.add(gitExecutablePath);
commands.add("describe"); //$NON-NLS-1$

ProcessBuilderFactory processRunner = new ProcessBuilderFactory();
try
{
Logger.log("Executing commads: " + commands.toString());
Map<String, String> environment = new HashMap<>(System.getenv());
IStatus status = processRunner.runInBackground(commands, Path.fromOSString(idfPath), environment);
if (status == null)
{
Logger.log(IDFCorePlugin.getPlugin(),
IDFCorePlugin.errorStatus("Unable to get the process status.", null)); //$NON-NLS-1$
return StringUtil.EMPTY;
}
String cmdOutput = status.getMessage();
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have check for status == null but we are not using break in that condition, so here is possible NPE

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

String version = StringUtil.EMPTY;
String patternString = "\\d+\\.\\d+\\.\\d+"; //$NON-NLS-1$
Pattern pattern = Pattern.compile(patternString);
Matcher matcher = pattern.matcher(cmdOutput);
if (matcher.find())
{
version = matcher.group(0);
}

if(StringUtil.isEmpty(version))
{
return StringUtil.EMPTY;
}

return version;
}
catch (Exception e)
{
Logger.log(e);
}

return StringUtil.EMPTY;
}

private boolean isVersionGreaterOrEqual(String version, String minVersion)
{
// Define the regex pattern to match version numbers
String pattern = "\\d+\\.\\d+\\.\\d+"; //$NON-NLS-1$
Pattern r = Pattern.compile(pattern);

// Now create matcher object.
Matcher m = r.matcher(version);
Matcher mMin = r.matcher(minVersion);

if (m.find() && mMin.find())
{
String[] currentVersionParts = m.group(0).split("\\."); //$NON-NLS-1$
String[] minVersionParts = mMin.group(0).split("\\."); //$NON-NLS-1$

int currentMajor = Integer.parseInt(currentVersionParts[0]);
int currentMinor = Integer.parseInt(currentVersionParts[1]);
int currentPatch = Integer.parseInt(currentVersionParts[2]);

int minMajor = Integer.parseInt(minVersionParts[0]);
int minMinor = Integer.parseInt(minVersionParts[1]);
int minPatch = Integer.parseInt(minVersionParts[2]);

if (currentMajor > minMajor)
{
return true;
}
else if (currentMajor == minMajor)
{
if (currentMinor > minMinor)
{
return true;
}
else if (currentMinor == minMinor)
{
return currentPatch >= minPatch;
}
}
}

return false;
}


protected IDFVersion Version()
{
String versionTxt = versionCombo.getText();
IDFVersion version = versionsMap.get(versionTxt);
return version;

Check warning on line 579 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

NM_METHOD_NAMING_CONVENTION 

The method name com.espressif.idf.ui.install.IDFDownloadPage.Version() doesn't start with a lower case letter
Raw output 
Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized.
}

public String getDestinationLocation()
Expand All @@ -476,24 +593,6 @@
{
return fileSystemBtn.getSelection();
}

private void showMessage()
{
Display.getDefault().asyncExec(new Runnable()
{

@Override
public void run() {
boolean allowToCreateProjectWithSpaces = MessageDialog.openQuestion(Display.getDefault().getActiveShell(), Messages.IDFDownloadWizard_AllowSpacesTitle,
Messages.IDFDownloadWizard_AllowSpacesMsg);

if (allowToCreateProjectWithSpaces) {
setErrorMessage(null);
setPageComplete(true);
}
}
});
}

public String getPythonExePath()
{
Expand All @@ -514,64 +613,64 @@
private static final String PYTHON_FILTERS = "Python Executables"; //$NON-NLS-1$
private static final String WINDOWS_EXTENSION = ".exe"; //$NON-NLS-1$

private BrowseButtonSelectionAdapter(Text text, int dialog)
{
this.linkedText = text;
this.dialog = dialog;
}

@Override
public void widgetSelected(SelectionEvent selectionEvent)
{
FileDialog dlg = null;
if (dialog == GIT_TOOL)
{
dlg = gitDialog();
}
else
{
dlg = pythonDialog();
}

dlg.setText(Messages.FileSelectionDialogTitle);
String dir = dlg.open();
if (!StringUtil.isEmpty(dir))
{
linkedText.setText(dir);
}
}

private FileDialog pythonDialog()
{
FileDialog dialog = new FileDialog(PlatformUI.getWorkbench().getDisplay().getActiveShell());
if (Platform.getOS().equals(Platform.OS_WIN32))
{
dialog.setFilterNames(new String[] { PYTHON_FILTERS });
dialog.setFilterExtensions(new String[] { PYTHON_FILE.concat(WINDOWS_EXTENSION) });
}
else
{
dialog.setFilterNames(new String[] { PYTHON_FILTERS });
dialog.setFilterExtensions(new String[] { PYTHON_FILE });
}
return dialog;
}

private FileDialog gitDialog()
{
FileDialog dialog = new FileDialog(PlatformUI.getWorkbench().getDisplay().getActiveShell());
if (Platform.getOS().equals(Platform.OS_WIN32))
{
dialog.setFilterNames(new String[] { GIT_FILE.concat(WINDOWS_EXTENSION) });
dialog.setFilterExtensions(new String[] { GIT_FILE.concat(WINDOWS_EXTENSION) });
}
else
{
dialog.setFilterNames(new String[] { GIT_FILE });
dialog.setFilterExtensions(new String[] { GIT_FILE });
}

return dialog;

Check warning on line 673 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

SIC_INNER_SHOULD_BE_STATIC 

Should com.espressif.idf.ui.install.IDFDownloadPage$BrowseButtonSelectionAdapter 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 All @@ -587,13 +686,13 @@
@Override
public void modifyText(ModifyEvent e)
{
switch (tool)
{
case GIT_TOOL:
gitExecutablePath = gitText.getText();
break;
case PYTHON_TOOL:
pythonExecutablePath = pythonText.getText();

Check warning on line 695 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/IDFDownloadPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

SF_SWITCH_NO_DEFAULT 

Switch statement found in com.espressif.idf.ui.install.IDFDownloadPage$ModifyTextValidationListener.modifyText(ModifyEvent) where default case is missing
Raw output 
This method contains a switch statement where default case is missing. Usually you need to provide a default case.

Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.
default:
break;
}
Expand Down
3 changes: 2 additions & 1 deletion bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class Messages extends NLS
public static String IDFDownloadPage_0;
public static String IDFDownloadPage_BrowseBtn;
public static String IDFDownloadPage_BrowseBtnTxt;
public static String IDFDownloadPage_CantfindIDFpy;
public static String IDFDownloadPage_CantfindProperEspIDFDirectory;
public static String IDFDownloadPage_CantFindRequirementsFile;
public static String IDFDownloadPage_ChooseAnExistingIDF;
public static String IDFDownloadPage_ChooseDirIDF;
Expand All @@ -22,6 +22,7 @@ public class Messages extends NLS
public static String IDFDownloadPage_DirectoryDialogMsg;
public static String IDFDownloadPage_DirectoryDialogText;
public static String IDFDownloadPage_DirectoryDialogTxt;
public static String IDFDownloadPage_VersionSpaceError;
public static String IDFDownloadPage_DownloadIDF;
public static String IDFDownloadPage_IDFBuildNotSupported;
public static String IDFDownloadPage_Note;
Expand Down
3 changes: 2 additions & 1 deletion bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/install/messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ GitRepositoryBuilder_gitClone=git clone result: %s
IDFDownloadPage_0=icons/espressif_logo.png
IDFDownloadPage_BrowseBtn=Browse...
IDFDownloadPage_BrowseBtnTxt=Browse...
IDFDownloadPage_CantfindIDFpy=Can not find idf.py in {0} tools
IDFDownloadPage_CantfindProperEspIDFDirectory=Can not find idf python scripts in {0} tools. Make sure to select valid ESP-IDF directory.
IDFDownloadPage_ChooseAnExistingIDF=Use an existing ESP-IDF directory from file system
IDFDownloadPage_ChooseDirIDF=Choose existing ESP-IDF directory:
IDFDownloadPage_ChooseIDFDir=Choose a directory to download ESP-IDF to:
IDFDownloadPage_ChooseIDFVersion=Please choose ESP-IDF version to download:
IDFDownloadPage_ClickFinishToDownload=Click on `Finish` to download
IDFDownloadPage_ClickOnFinish=Click on `Finish` to configure IDF_PATH with
IDFDownloadPage_DirDoesnotExist=Directory doesn''t exist:
IDFDownloadPage_VersionSpaceError=ESP-IDF build system support spaces in paths after v5.0. Please choose a different directory.
IDFDownloadPage_DirectoryDialogMessage=Choose Directory to download ESP-IDF
IDFDownloadPage_DirectoryDialogMsg=Select ESP-IDF Directory:
IDFDownloadPage_DirectoryDialogText=Choose Directory
Expand Down
25 changes: 9 additions & 16 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,31 +107,24 @@

}

idfToolSet.getEnvVars().put(IDFEnvironmentVariables.IDF_COMPONENT_MANAGER, "1");
idfToolSet.getEnvVars().put(IDFEnvironmentVariables.IDF_COMPONENT_MANAGER, "1"); //$NON-NLS-1$
// IDF_MAINTAINER=1 to be able to build with the clang toolchain
idfToolSet.getEnvVars().put(IDFEnvironmentVariables.IDF_MAINTAINER, "1");
idfToolSet.getEnvVars().put(IDFEnvironmentVariables.IDF_MAINTAINER, "1"); //$NON-NLS-1$
if (!StringUtil.isEmpty(idfPath))
{
idfToolSet.getEnvVars().put(IDFEnvironmentVariables.IDF_PATH, idfPath);
idfToolSet.setIdfLocation(idfPath);
}

if (StringUtil.isEmpty(idfToolSet.getEnvVars().get(IDFEnvironmentVariables.ESP_IDF_VERSION)))
IStatus status = getIdfVersionFromIdfPy();
String cmdOutput = status.getMessage();
Pattern pattern = Pattern.compile("v(\\d+\\.\\d+\\.\\d+)"); //$NON-NLS-1$
Matcher matcher = pattern.matcher(cmdOutput.toLowerCase());
if (matcher.find())
{
IStatus status = getIdfVersionFromIdfPy();
String cmdOutput = status.getMessage();
Pattern pattern = Pattern.compile("v(\\d+\\.\\d+\\.\\d+)");
Matcher matcher = pattern.matcher(cmdOutput.toLowerCase());
if (matcher.find())
{
idfToolSet.setIdfVersion(matcher.group(1));
}
idfToolSet.getEnvVars().put(IDFEnvironmentVariables.ESP_IDF_VERSION, idfToolSet.getIdfVersion());
}
else
{
idfToolSet.setIdfVersion(idfToolSet.getEnvVars().get(IDFEnvironmentVariables.ESP_IDF_VERSION));
idfToolSet.setIdfVersion(matcher.group(1));
}
idfToolSet.getEnvVars().put(IDFEnvironmentVariables.ESP_IDF_VERSION, idfToolSet.getIdfVersion());

}

Expand Down Expand Up @@ -357,7 +350,7 @@
addPathToEnvironmentPath(environment, gitExecutablePath);
}
Process process = processRunner.run(arguments, org.eclipse.core.runtime.Path.ROOT, environment);
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));

Check warning on line 353 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.ui.tools.ToolsJob.runCommandIdfPyInIdfEnv(List, MessageConsoleStream): new java.io.InputStreamReader(InputStream)
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.

Check warning on line 353 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java

View workflow job for this annotation

GitHub Actions / spotbugs

OS_OPEN_STREAM 

com.espressif.idf.ui.tools.ToolsJob.runCommandIdfPyInIdfEnv(List, MessageConsoleStream) may fail to close stream
Raw output 
The method creates an IO stream object, does not assign it to any fields, pass it to other methods that might close it, or return it, and does not appear to close the stream on all paths out of the method.  This may result in a file descriptor leak.  It is generally a good idea to use a finally block to ensure that streams are closed.
String line;
while ((line = reader.readLine()) != null)
{
Expand Down Expand Up @@ -543,11 +536,11 @@
{
errorThread.interrupt();
}
if (readerThread != null)

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

View workflow job for this annotation

GitHub Actions / spotbugs

RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE 

Redundant nullcheck of readerThread, which is known to be non-null in com.espressif.idf.ui.tools.ToolsJob.processData(Process)
Raw output 
This method contains a redundant check of a known non-null value against the constant null.
{
readerThread.join();
}
if (errorThread != null)

Check warning on line 543 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/ToolsJob.java

View workflow job for this annotation

GitHub Actions / spotbugs

RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE 

Redundant nullcheck of errorThread, which is known to be non-null in com.espressif.idf.ui.tools.ToolsJob.processData(Process)
Raw output 
This method contains a redundant check of a known non-null value against the constant null.
{
errorThread.join();
}
Expand Down
1 change: 0 additions & 1 deletion ...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 @@ -36,7 +36,6 @@
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 Down Expand Up @@ -76,7 +75,7 @@
GridLayout gridLayout = new GridLayout(numColumns, false);
container.setLayout(gridLayout);
createIdfTable(container);
return container;

Check warning on line 78 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 refreshEditorUI()
Expand Down Expand Up @@ -487,56 +486,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 538 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
Loading