Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.core/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Bundle-ClassPath: .,
lib/commons-compress-1.21.jar,
lib/xz-1.9.jar
Import-Package: org.apache.commons.logging,
org.eclipse.cdt.debug.core,
org.eclipse.cdt.debug.core.launch,
org.eclipse.embedcdt.core,
org.eclipse.ui.console

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
public class IDFBuildConfiguration extends CBuildConfiguration
{

private static final ActiveLaunchConfigurationProvider LAUNCH_CONFIG_PROVIDER = new ActiveLaunchConfigurationProvider();
private static final String NINJA = "Ninja"; //$NON-NLS-1$
protected static final String COMPILE_COMMANDS_JSON = "compile_commands.json"; //$NON-NLS-1$
protected static final String COMPONENTS = "components"; //$NON-NLS-1$
Expand Down Expand Up @@ -190,16 +189,29 @@ public String getProperty(String name)
{
try
{
ILaunchConfiguration configuration = LAUNCH_CONFIG_PROVIDER.getActiveLaunchConfiguration();
if (configuration != null
&& configuration.getType().getIdentifier().equals(IDFLaunchConstants.DEBUG_LAUNCH_CONFIG_TYPE))
ILaunchBarManager launchBarManager = IDFCorePlugin.getService(ILaunchBarManager.class);
ILaunchConfiguration configuration = null;

if (launchBarManager != null)
{
configuration = launchBarManager.getActiveLaunchConfiguration();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (configuration == null)
{
Logger.log("Warning: Launch Bar not ready. Falling back to default properties for " + name); //$NON-NLS-1$
return super.getProperty(name);
Comment on lines +200 to +203
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Avoid warning-level logging on this hot path.

getProperty() is called a lot, and configuration == null is a normal transient state during Launch Bar startup. Emitting a warning here can easily replace the old startup noise with a new one. Please downgrade this or log it once.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java`
around lines 200 - 203, The warning in IDFBuildConfiguration.getProperty logs
every time configuration == null which is noisy; change it to a lower log level
or log it only once. Update the branch that currently calls Logger.log("Warning:
Launch Bar not ready...") so it either uses a debug/trace/info call instead of
warning, or guard it with a one-time flag (e.g., a private static
boolean/AtomicBoolean) so the message is emitted only once, then continue to
return super.getProperty(name) as before; reference:
IDFBuildConfiguration.getProperty, the configuration variable and the return
super.getProperty(name) line.

}

if (configuration.getType().getIdentifier().equals(IDFLaunchConstants.DEBUG_LAUNCH_CONFIG_TYPE))
{
configuration = new LaunchUtil(DebugPlugin.getDefault().getLaunchManager())
.getBoundConfiguration(configuration);
}
String property = configuration == null ? StringUtil.EMPTY
: configuration.getAttribute(name, StringUtil.EMPTY);

String property = configuration.getAttribute(name, StringUtil.EMPTY);
property = property.isBlank() ? getSettings().get(name, StringUtil.EMPTY) : property;
Comment on lines +200 to 213
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve the existing settings fallback when no active launch configuration exists.

This early return changes the resolution order from “launch config, then getSettings()” to super.getProperty(name) whenever Launch Bar is not ready. That means persisted IDF build properties can disappear during startup even though the fallback logic is still present a few lines below.

Suggested fix
 			if (configuration == null)
 			{
-				Logger.log("Warning: Launch Bar not ready. Falling back to default properties for " + name); //$NON-NLS-1$
-				return super.getProperty(name);
+				Logger.log("Warning: Launch Bar not ready. Falling back to persisted properties for " + name); //$NON-NLS-1$
+				String property = getSettings().get(name, StringUtil.EMPTY);
+				return property.isBlank() ? super.getProperty(name) : property;
 			}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java`
around lines 200 - 213, The early return in IDFBuildConfiguration.getProperty
when configuration == null skips the intended fallback to persisted settings;
change the logic so that when configuration is null you do not return
super.getProperty(name) immediately but instead let the method continue to
compute property by first checking the (absent) launch configuration and then
falling back to getSettings().get(name, StringUtil.EMPTY) (or finally
super.getProperty(name) only if both configuration and getSettings() produce
empty), i.e., remove the immediate return and ensure getSettings() is consulted
before delegating to super.getProperty; update references in
IDFBuildConfiguration.getProperty and preserve the existing resolution order
(launch config → getSettings() → super.getProperty).


return property;
}
catch (CoreException e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
import org.eclipse.debug.core.DebugPlugin;
import org.eclipse.debug.core.ILaunchConfiguration;
import org.eclipse.debug.core.ILaunchManager;
import org.eclipse.launchbar.core.ILaunchBarManager;
import org.eclipse.ui.console.MessageConsole;
import org.eclipse.ui.console.MessageConsoleStream;

import com.espressif.idf.core.IDFCorePlugin;
import com.espressif.idf.core.IDFEnvironmentVariables;
import com.espressif.idf.core.ProcessBuilderFactory;
import com.espressif.idf.core.build.ActiveLaunchConfigurationProvider;
import com.espressif.idf.core.build.IDFLaunchConstants;
import com.espressif.idf.core.logging.Logger;

Expand All @@ -36,8 +36,7 @@ public class IdfCommandExecutor

private final String target;
private final MessageConsole console;
private static final String CMAKE_ARGUMENTS = "cmake.arguments"; //$NON-NLS-1$
private static final ActiveLaunchConfigurationProvider LAUNCH_CONFIG_PROVIDER = new ActiveLaunchConfigurationProvider();


public IdfCommandExecutor(String target, MessageConsole console)
{
Expand Down Expand Up @@ -100,7 +99,14 @@ public String getProperty(String name)
{
try
{
ILaunchConfiguration configuration = LAUNCH_CONFIG_PROVIDER.getActiveLaunchConfiguration();
ILaunchBarManager launchBarManager = IDFCorePlugin.getService(ILaunchBarManager.class);
ILaunchConfiguration configuration = null;

if (launchBarManager != null)
{
configuration = launchBarManager.getActiveLaunchConfiguration();
}

if (configuration != null
&& configuration.getType().getIdentifier().equals(IDFLaunchConstants.DEBUG_LAUNCH_CONFIG_TYPE))
{
Expand Down

This file was deleted.

Loading