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-1089 , IEP-1062: Project Creation Page updated #846

Merged
merged 12 commits into from
Nov 22, 2023
Merged

IEP-1089 , IEP-1062: Project Creation Page updated #846

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7b2eb1d
IEP-1089: Updated the project creation wizard to have only one page w…
alirana01 Nov 2, 2023
dbb9ea6
Fix for project ui tests for new ui
alirana01 Nov 2, 2023
692bc3f
Minor code review things from the coderabbitai review
alirana01 Nov 2, 2023
2a61f62
added the esp target selection to the project
alirana01 Nov 6, 2023
177aeb5
unwanted import removed
alirana01 Nov 6, 2023
001a800
Updated UI for the grouped templates
alirana01 Nov 8, 2023
1601a97
fixed issue related to hidden control on some UIs
alirana01 Nov 8, 2023
68a9caa
review comments for code improvement
alirana01 Nov 8, 2023
71e9020
fix for the finish button
alirana01 Nov 13, 2023
d710d91
updated to handle the finish button and target change bug
alirana01 Nov 14, 2023
787fccd
improved the job join
alirana01 Nov 14, 2023
70b89b0
coderabbit review comments resolved
alirana01 Nov 15, 2023
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
7 changes: 4 additions & 3 deletions bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/EspConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
alirana01 marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -32,12 +34,11 @@
}

@SuppressWarnings("unchecked")
public List<String> getTargets()
public Set<String> getTargets()
{

List<String> targets = new ArrayList<String>();
Set<String> targets = new LinkedHashSet<String>();
JSONParser jsonParser = new JSONParser();
try (FileReader reader = new FileReader(espConfigPath))

Check warning on line 41 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/EspConfigParser.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.util.EspConfigParser.getTargets(): 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.
{
JSONObject json = (JSONObject) jsonParser.parse(reader);
JSONArray targetsArray = (JSONArray) json.get("targets"); //$NON-NLS-1$
Expand Down Expand Up @@ -66,7 +67,7 @@
List<String> voltages = new ArrayList<String>();
JSONObject voltageOption;
JSONParser jsonParser = new JSONParser();
try (FileReader reader = new FileReader(espConfigPath))

Check warning on line 70 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/EspConfigParser.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.util.EspConfigParser.getEspFlashVoltages(): 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.
{
JSONObject json = (JSONObject) jsonParser.parse(reader);
JSONArray optionsArray = (JSONArray) json.get("options"); //$NON-NLS-1$
Expand Down Expand Up @@ -99,7 +100,7 @@
Map<String, JSONArray> boardsConfigs = new HashMap<>();
List<JSONObject> objects = new ArrayList<>();
JSONParser jsonParser = new JSONParser();
try (FileReader reader = new FileReader(espConfigPath))

Check warning on line 103 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/EspConfigParser.java

View workflow job for this annotation

GitHub Actions / spotbugs

DM_DEFAULT_ENCODING 

Found reliance on default encoding in com.espressif.idf.core.util.EspConfigParser.getBoardsConfigs(String): 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.
{
JSONObject json = (JSONObject) jsonParser.parse(reader);
JSONArray boardsArray = (JSONArray) json.get("boards"); //$NON-NLS-1$
Expand Down
65 changes: 58 additions & 7 deletions ...m.espressif.idf.ui/src/com/espressif/idf/ui/templates/AbstractTemplatesSelectionPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*******************************************************************************/
package com.espressif.idf.ui.templates;

import static org.eclipse.swt.events.SelectionListener.widgetSelectedAdapter;

import java.io.IOException;
import java.util.Iterator;

Expand All @@ -24,8 +26,9 @@
import org.eclipse.swt.custom.SashForm;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Group;
import org.eclipse.ui.dialogs.FilteredTree;
import org.eclipse.ui.dialogs.PatternFilter;

Expand All @@ -43,6 +46,8 @@
protected ITemplateNode templateElements;
protected FilteredTree filteredTree;
private WizardSelectedAction doubleClickAction = new WizardSelectedAction();
private Group templateGroup;
private Button fUseTemplate;

private class WizardSelectedAction extends Action
{
Expand Down Expand Up @@ -85,12 +90,18 @@
container.setLayoutData(new GridData(GridData.FILL_BOTH));

createAbove(container, 1);
Label label = new Label(container, SWT.NONE);
label.setText(getLabel());
GridData gd = new GridData();
label.setLayoutData(gd);
if (templateElements == null || templateElements.getChildren().isEmpty())
{
Dialog.applyDialogFont(container);
setControl(container);
return;
}
alirana01 marked this conversation as resolved.
Show resolved Hide resolved

SashForm sashForm = new SashForm(container, SWT.HORIZONTAL);
GridData gd = new GridData();

Check warning on line 100 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/AbstractTemplatesSelectionPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

DLS_DEAD_LOCAL_STORE 

Dead store to gd in com.espressif.idf.ui.templates.AbstractTemplatesSelectionPage.createControl(Composite)
Raw output 
This instruction assigns a value to a local variable, but the value is not read or used in any subsequent instruction. Often, this indicates an error, because the value computed is never used.

Note that Sun's javac compiler often generates dead stores for final local variables. Because SpotBugs is a bytecode-based tool, there is no easy way to eliminate these false positives.

templateGroup = new Group(container, SWT.NONE);
fUseTemplate = new Button(templateGroup, SWT.CHECK);
SashForm sashForm = new SashForm(templateGroup, SWT.HORIZONTAL);
gd = new GridData(GridData.FILL_BOTH);
// limit the width of the sash form to avoid the wizard
// opening very wide. This is just preferred size -
Expand All @@ -99,6 +110,33 @@
gd.widthHint = 400;
gd.heightHint = 350;
sashForm.setLayoutData(gd);

templateGroup.setText(Messages.TemplateGroupHeader);
GridLayout groupLayout = new GridLayout();
templateGroup.setLayout(groupLayout);
GridData gridDataGp = new GridData(GridData.FILL_BOTH);
gridDataGp.widthHint = 400;
gridDataGp.heightHint = 350;
templateGroup.setLayoutData(gridDataGp);



fUseTemplate.setText(Messages.TemplateListSelectionPage_SelectTemplate_Desc);
GridData gridData = new GridData();
gridData.horizontalSpan = 1;
fUseTemplate.setLayoutData(gridData);
fUseTemplate.addSelectionListener(widgetSelectedAdapter(e -> {
templateViewer.getControl().setEnabled(fUseTemplate.getSelection());
filteredTree.setEnabled(fUseTemplate.getSelection());
if (!fUseTemplate.getSelection())
setDescription(""); //$NON-NLS-1$
else
setDescription(Messages.TemplateListSelectionPage_Template_Wizard_Desc);

setDescriptionEnabled(fUseTemplate.getSelection());
getContainer().updateButtons();
}));
fUseTemplate.setSelection(false);

templateViewer = createTreeViewer(sashForm);
templateViewer.setContentProvider(new TemplatesContentProvider());
Expand All @@ -107,7 +145,10 @@
templateViewer.addDoubleClickListener(event -> doubleClickAction.run());
createDescriptionIn(sashForm);
createBelow(container, 1);
templateViewer.setInput(templateElements);
if (templateElements != null && !templateElements.getChildren().isEmpty())

Check warning on line 148 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/AbstractTemplatesSelectionPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE 

Redundant nullcheck of com.espressif.idf.ui.templates.AbstractTemplatesSelectionPage.templateElements, which is known to be non-null in com.espressif.idf.ui.templates.AbstractTemplatesSelectionPage.createControl(Composite)
Raw output 
This method contains a redundant check of a known non-null value against the constant null.
{
templateViewer.setInput(templateElements);
}
initializeViewer();
templateViewer.addSelectionChangedListener(this);

Expand Down Expand Up @@ -230,4 +271,14 @@
IStructuredSelection ssel = templateViewer.getStructuredSelection();
return ssel != null && !ssel.isEmpty();
}

public Button getfUseTemplate()
{
return fUseTemplate;

Check warning on line 277 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/AbstractTemplatesSelectionPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP 

com.espressif.idf.ui.templates.AbstractTemplatesSelectionPage.getfUseTemplate() may expose internal representation by returning AbstractTemplatesSelectionPage.fUseTemplate
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 setfUseTemplate(Button fUseTemplate)
{
this.fUseTemplate = fUseTemplate;

Check warning on line 282 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/AbstractTemplatesSelectionPage.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP2 

com.espressif.idf.ui.templates.AbstractTemplatesSelectionPage.setfUseTemplate(Button) may expose internal representation by storing an externally mutable object into AbstractTemplatesSelectionPage.fUseTemplate
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.
}
alirana01 marked this conversation as resolved.
Show resolved Hide resolved
}
44 changes: 43 additions & 1 deletion bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/IDFProjectGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.Path;
import org.eclipse.launchbar.core.ILaunchBarManager;
import org.eclipse.launchbar.core.target.ILaunchTarget;
import org.eclipse.launchbar.core.target.ILaunchTargetManager;
import org.eclipse.ui.console.MessageConsoleStream;
import org.osgi.framework.Bundle;

import com.espressif.idf.core.IDFConstants;
import com.espressif.idf.core.IDFProjectNature;
import com.espressif.idf.core.build.IDFLaunchConstants;
import com.espressif.idf.core.logging.Logger;
import com.espressif.idf.core.util.FileUtil;
import com.espressif.idf.ui.UIPlugin;
Expand All @@ -37,12 +42,17 @@

private File sourceTemplatePath;
private boolean copyIntoWorkspace;
private String target;
protected MessageConsoleStream console;
private ILaunchBarManager launchBarManager;

public IDFProjectGenerator(String manifestFile, File source, boolean copyIntoWorkspace)
public IDFProjectGenerator(String manifestFile, File source, boolean copyIntoWorkspace, String target)
{
super(manifestFile);
this.sourceTemplatePath = source;
this.copyIntoWorkspace = copyIntoWorkspace;
this.target = target;
launchBarManager = UIPlugin.getService(ILaunchBarManager.class);
}

@Override
Expand All @@ -66,6 +76,9 @@
Logger.log("Source Template path:" + sourceTemplatePath); //$NON-NLS-1$
if (sourceTemplatePath == null)
{
setTarget();
// refresh to see the copied resources in the project explorer
getProject().refreshLocal(IResource.DEPTH_INFINITE, monitor);
return; // let's go with the default generate
}

Expand All @@ -85,8 +98,36 @@
}
}

setTarget();

// refresh to see the copied resources in the project explorer
project.refreshLocal(IResource.DEPTH_INFINITE, monitor);

}

private void setTarget() throws CoreException
{
ILaunchTarget launchTarget = findSuitableTargetForSelectedTargetString();
launchBarManager.setActiveLaunchTarget(launchTarget);
}

private ILaunchTarget findSuitableTargetForSelectedTargetString()
{
ILaunchTargetManager launchTargetManager = UIPlugin.getService(ILaunchTargetManager.class);
ILaunchTarget[] targets = launchTargetManager
.getLaunchTargetsOfType(IDFLaunchConstants.ESP_LAUNCH_TARGET_TYPE);

for (ILaunchTarget iLaunchTarget : targets)
{
String idfTarget = iLaunchTarget.getAttribute(IDFLaunchConstants.ATTR_IDF_TARGET,
null);
if (idfTarget.contentEquals(target))
{
return iLaunchTarget;
}
}

return null;
}
alirana01 marked this conversation as resolved.
Show resolved Hide resolved

@Override
Expand All @@ -110,7 +151,7 @@
File projectFile = targetProject.getLocation().toFile();
File[] files = sourceTemplateFile.listFiles();

for (File file : files)

Check warning on line 154 in bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/IDFProjectGenerator.java

View workflow job for this annotation

GitHub Actions / spotbugs

NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE 

Possible null pointer dereference in com.espressif.idf.ui.templates.IDFProjectGenerator.copyIDFTemplateToWorkspace(String, File, IProject) due to return value of called method
Raw output 
The return value from a method is dereferenced without a null check, and the return value of that method is one that should generally be checked for null. This may lead to a NullPointerException when the code is executed.
{
// Don't copy build folder as CMakeCache.txt file contains full path entries and leads to build issues.
if (file.getName().equals(IDFConstants.BUILD_FOLDER))
Expand All @@ -130,4 +171,5 @@
}
}
}

}
9 changes: 7 additions & 2 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/templates/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@ public class Messages extends NLS
public static String BaseWizardSelectionPage_No_Desc;
public static String TemplateListSelectionPage_SelectTemplate_Desc;
public static String TemplateListSelectionPage_Template_Wizard_Desc;
public static String TemplateListSelectionPage_Templates;
public static String TemplateListSelectionPage_Templates_Desc;
public static String NewProjectWizardPage_Header;
public static String NewProjectWizardPage_DescriptionString;
public static String TemplateSelectionToolTip;
public static String NewProjectWizardPage_NoTemplateFoundMessage;
public static String TemplateGroupHeader;
public static String NewProjectTargetSelection_Tooltip;
public static String NewProjectTargetSelection_Label;

static
{
// initialize resource bundle
Expand Down
Loading
Loading