Skip to content

Commit 72c5644

Browse files
authored
fix - Terminal integration issues (#1145)
* fix the bug Not properly disposed SWT resource. * fix the bug that failed to install terminal bundle.
1 parent 13dd516 commit 72c5644

5 files changed

Lines changed: 33 additions & 12 deletions

File tree

com.microsoft.copilot.eclipse.terminal.api/src/com/microsoft/copilot/eclipse/terminal/api/IRunInTerminalTool.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
import java.util.Map;
44
import java.util.concurrent.CompletableFuture;
55

6-
import org.eclipse.swt.graphics.Image;
6+
import org.eclipse.jface.resource.ImageDescriptor;
7+
78

89
/**
910
* Interface for terminal tool implementations that can execute commands in a terminal. This interface is used by the
@@ -38,7 +39,7 @@ public interface IRunInTerminalTool {
3839
public StringBuilder getBackgroundCommandOutput(String executionId);
3940

4041
/**
41-
* Sets the terminal icon for the tool.
42+
* Sets the terminal icon descriptor for the tool.
4243
*/
43-
public void setTerminalIcon(Image terminalIcon);
44+
public void setTerminalIconDescriptor(ImageDescriptor terminalIconDescriptor);
4445
}

com.microsoft.copilot.eclipse.terminal.api/src/com/microsoft/copilot/eclipse/terminal/api/TerminalServiceManager.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,15 @@ private IStatus installAndRefreshBundle(String bundleLocation, IProgressMonitor
249249
}
250250
monitor.worked(20);
251251
monitor.subTask("Installing bundle");
252-
Bundle installedBundle = bundleContext.installBundle(bundleLocation);
252+
Bundle installedBundle;
253+
try {
254+
installedBundle = bundleContext.installBundle(bundleLocation);
255+
} catch (Exception e) {
256+
CopilotCore.LOGGER.error("Failed to install terminal bundle from location: " + bundleLocation, e);
257+
monitor.done();
258+
return new Status(IStatus.ERROR, bundleContext.getBundle().getSymbolicName(),
259+
"Failed to install terminal bundle: " + e.getMessage(), e);
260+
}
253261
Set<Bundle> bundlesToStart = new HashSet<>();
254262
Set<Bundle> toRefresh = new HashSet<>();
255263
bundlesToStart.add(installedBundle);
@@ -327,7 +335,7 @@ private URL getBundleResourceUrl(String bundleJarName) {
327335
if ("com.microsoft.copilot.eclipse.ui".equals(bundle.getSymbolicName())) {
328336
URL bundleUrl = FileLocator.find(bundle, new Path(TERMINAL_BUNDLES_PATH + bundleJarName));
329337
if (bundleUrl != null) {
330-
return FileLocator.resolve(bundleUrl);
338+
return FileLocator.toFileURL(bundleUrl);
331339
}
332340
}
333341
}

com.microsoft.copilot.eclipse.ui.terminal.tm/src/com/microsoft/copilot/eclipse/ui/terminal/tm/RunInTerminalTool.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.apache.commons.lang3.StringUtils;
1010
import org.eclipse.core.runtime.Platform;
11+
import org.eclipse.jface.resource.ImageDescriptor;
1112
import org.eclipse.swt.custom.CTabFolder;
1213
import org.eclipse.swt.custom.CTabItem;
1314
import org.eclipse.swt.events.DisposeListener;
@@ -50,6 +51,7 @@ public class RunInTerminalTool implements IRunInTerminalTool {
5051
private CTabFolder tabFolder;
5152
private CTabItem copilotTabItem;
5253
private Image terminalIcon;
54+
private ImageDescriptor terminalIconDescriptor;
5355

5456
// Output and command state
5557
private StringBuilder sb;
@@ -185,7 +187,10 @@ private ITerminalControl getTerminalControl(String terminalTitle, boolean isBack
185187
if (tabFolder != null) {
186188
for (CTabItem item : tabFolder.getItems()) {
187189
if (terminalTitle.equals(item.getText())) {
188-
if (terminalIcon != null) {
190+
if (terminalIconDescriptor != null) {
191+
if (terminalIcon == null || terminalIcon.isDisposed()) {
192+
terminalIcon = terminalIconDescriptor.createImage();
193+
}
189194
item.setImage(terminalIcon);
190195
}
191196
item.addDisposeListener(
@@ -321,7 +326,7 @@ private IWorkbenchPage getActivePage() {
321326
}
322327

323328
@Override
324-
public void setTerminalIcon(Image terminalIcon) {
325-
this.terminalIcon = terminalIcon;
329+
public void setTerminalIconDescriptor(ImageDescriptor iconDescriptor) {
330+
this.terminalIconDescriptor = iconDescriptor;
326331
}
327332
}

com.microsoft.copilot.eclipse.ui.terminal/src/com/microsoft/copilot/eclipse/ui/terminal/RunInTerminalTool.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.apache.commons.lang3.StringUtils;
1010
import org.eclipse.core.runtime.Platform;
11+
import org.eclipse.jface.resource.ImageDescriptor;
1112
import org.eclipse.swt.custom.CTabFolder;
1213
import org.eclipse.swt.custom.CTabItem;
1314
import org.eclipse.swt.events.DisposeListener;
@@ -50,6 +51,7 @@ public class RunInTerminalTool implements IRunInTerminalTool {
5051
private CTabFolder tabFolder;
5152
private CTabItem copilotTabItem;
5253
private Image terminalIcon;
54+
private ImageDescriptor terminalIconDescriptor;
5355

5456
// Output and command state
5557
private StringBuilder sb;
@@ -186,7 +188,10 @@ private ITerminalControl getTerminalControl(String terminalTitle, boolean isBack
186188
if (tabFolder != null) {
187189
for (CTabItem item : tabFolder.getItems()) {
188190
if (terminalTitle.equals(item.getText())) {
189-
if (terminalIcon != null) {
191+
if (terminalIconDescriptor != null) {
192+
if (terminalIcon == null || terminalIcon.isDisposed()) {
193+
terminalIcon = terminalIconDescriptor.createImage();
194+
}
190195
item.setImage(terminalIcon);
191196
}
192197
item.addDisposeListener(
@@ -321,8 +326,9 @@ private IWorkbenchPage getActivePage() {
321326
return null;
322327
}
323328

329+
324330
@Override
325-
public void setTerminalIcon(Image terminalIcon) {
326-
this.terminalIcon = terminalIcon;
331+
public void setTerminalIconDescriptor(ImageDescriptor iconDescriptor) {
332+
this.terminalIconDescriptor = iconDescriptor;
327333
}
328334
}

com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/tools/RunInTerminalToolAdapter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ public CompletableFuture<LanguageModelToolResult[]> invoke(Map<String, Object> i
117117
} else if (isBackgroundObj instanceof String) {
118118
isBackground = Boolean.parseBoolean((String) isBackgroundObj);
119119
}
120-
impl.setTerminalIcon(UiUtils.buildImageFromPngPath("/icons/github_copilot.png"));
120+
121+
impl.setTerminalIconDescriptor(UiUtils.buildImageDescriptorFromPngPath("/icons/github_copilot.png"));
121122

122123
return impl.executeCommand(command, isBackground)
123124
.thenApply(result -> new LanguageModelToolResult[] { new LanguageModelToolResult(result) })

0 commit comments

Comments
 (0)