Skip to content

Commit 5c5cbc0

Browse files
committed
Fix Eclipse plugin review issues
- Write the API key directly into ~/.devglobe/config.toml instead of running devglobe-core setup, so first-run setup works before the core binary is downloaded and the key never appears in the process list - Report activity on document edits, not just editor switches, so typing in a single file keeps you visible (the daemon stops sending heartbeats after a minute without activity) - Download the core off the UI thread so Connect no longer freezes Eclipse, and make the client field volatile - Actually send the shutdown message to the daemon on stop - Always use the win-x64 core on Windows since no win-arm64 build is published - Require JavaSE-17 to match the compile target so the bundle loads on a Java 17 Eclipse - Keep routine confirmations in the status line instead of modal dialogs, and clear it with timerExec instead of a thread per toast - Remove the status bar widget that was never registered, along with its README claim
1 parent 292e4cb commit 5c5cbc0

9 files changed

Lines changed: 119 additions & 122 deletions

File tree

eclipse-plugin/META-INF/MANIFEST.MF

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Bundle-SymbolicName: xyz.devglobe.eclipse; singleton:=true
55
Bundle-Version: 2.0.1.qualifier
66
Bundle-Activator: xyz.devglobe.eclipse.core.DevGlobePlugin
77
Bundle-Vendor: DevGlobe
8-
Bundle-RequiredExecutionEnvironment: JavaSE-21
8+
Bundle-RequiredExecutionEnvironment: JavaSE-17
99
Require-Bundle: org.eclipse.ui,
1010
org.eclipse.core.runtime,
1111
org.eclipse.swt,

eclipse-plugin/README.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ Visibility settings (anonymous mode, repo sharing, profile mode) are managed on
3939
| **Platform detection** | Sends your OS (macOS, Windows or Linux) alongside each heartbeat so it appears on your profile. |
4040
| **Git integration** | Detects your repo from the git remote. Commit data is never read or sent by the plugin. |
4141
| **Status message** | Write what you're working on — visible on your globe profile. Supports empty to clear. |
42-
| **Status bar** | Displays your coding time for today (e.g. `2h 15m`) in the Eclipse status bar. |
4342
| **Notifications** | Toast notifications in the Eclipse status line for connection events, errors, and status changes. |
4443

4544
### Sidebar
@@ -188,10 +187,9 @@ eclipse-plugin/
188187
│ ├── TrackerState.java # Immutable state data class
189188
│ └── LanguageService.java # File extension → language mapping
190189
├── auth/
191-
│ └── ConfigWriter.java # Spawns devglobe-core setup subprocess
190+
│ └── ConfigWriter.java # Reads/writes ~/.devglobe/config.toml
192191
├── ui/
193192
│ ├── DevGlobeView.java # Sidebar ViewPart (login + dashboard)
194-
│ ├── DevGlobeStatusBarContribution.java # Status bar widget
195193
│ ├── DocumentTracker.java # Editor activity listener
196194
│ └── Notifier.java # Toast notifications via status line
197195
└── actions/

eclipse-plugin/src/main/java/xyz/devglobe/eclipse/auth/ConfigWriter.java

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@
88
import java.util.regex.Matcher;
99
import java.util.regex.Pattern;
1010

11-
import xyz.devglobe.eclipse.core.CoreDownloader;
1211
import xyz.devglobe.eclipse.core.DevGlobePlugin;
1312

1413
/**
1514
* Reads and writes the DevGlobe config.toml file (~/.devglobe/config.toml).
16-
* API key writes delegate to {@code devglobe-core setup <key>} so the config
17-
* format always matches what the core expects.
15+
* The API key is written directly into the TOML root so setup works before
16+
* the core binary has been downloaded, and the key never appears on a process
17+
* command line.
1818
*/
1919
public final class ConfigWriter {
2020

@@ -37,34 +37,40 @@ public static File logPath() {
3737
// ── API Key ──────────────────────────────────────────────────────────
3838

3939
/**
40-
* Writes the API key by running {@code devglobe-core setup <key>}.
41-
* This ensures the config format always matches what the core expects.
40+
* Writes the API key into the {@code api_key} root entry of config.toml,
41+
* preserving any other settings and sections. Written with 0600 permissions.
4242
*/
4343
public static void writeApiKey(String apiKey) {
4444
try {
45-
File binary = CoreDownloader.getBinaryPath();
46-
if (!binary.exists()) {
47-
DevGlobePlugin.log("devglobe-core not found, cannot write API key");
48-
return;
49-
}
50-
51-
ProcessBuilder pb = new ProcessBuilder(
52-
binary.getAbsolutePath(), "setup", apiKey);
53-
pb.directory(devglobeDir());
54-
pb.redirectErrorStream(true);
45+
File dir = devglobeDir();
46+
if (!dir.exists()) dir.mkdirs();
5547

56-
Process process = pb.start();
57-
int exitCode = process.waitFor();
48+
File configFile = configPath();
49+
List<String> lines = configFile.exists()
50+
? new ArrayList<>(Files.readAllLines(configFile.toPath()))
51+
: new ArrayList<>();
5852

59-
if (exitCode != 0) {
60-
String error = new String(process.getInputStream().readAllBytes());
61-
DevGlobePlugin.log("devglobe-core setup failed (exit " + exitCode + "): " + error);
53+
String keyLine = "api_key = \"" + escapeToml(apiKey) + "\"";
54+
int idx = findRootApiKeyIndex(lines);
55+
if (idx >= 0) {
56+
lines.set(idx, keyLine);
57+
} else {
58+
lines.add(0, keyLine);
6259
}
63-
} catch (Exception e) {
64-
DevGlobePlugin.log("Failed to write API key via devglobe-core: " + e.getMessage());
60+
61+
String output = String.join("\n", lines).replaceAll("\n{3,}", "\n\n");
62+
if (!output.endsWith("\n")) output += "\n";
63+
Files.writeString(configFile.toPath(), output);
64+
setRestrictivePermissions(configFile);
65+
} catch (IOException e) {
66+
DevGlobePlugin.log("Failed to write API key: " + e.getMessage());
6567
}
6668
}
6769

70+
private static String escapeToml(String s) {
71+
return s.replace("\\", "\\\\").replace("\"", "\\\"");
72+
}
73+
6874
public static boolean hasApiKey() {
6975
try {
7076
File configFile = configPath();

eclipse-plugin/src/main/java/xyz/devglobe/eclipse/core/CoreClient.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package xyz.devglobe.eclipse.core;
22

33
import java.io.*;
4+
import java.util.concurrent.TimeUnit;
45

56
/**
67
* JSON-line IPC client for the devglobe-core subprocess.
@@ -42,7 +43,6 @@ public synchronized boolean start(String binaryPath) {
4243
try {
4344
ProcessBuilder pb = new ProcessBuilder(binaryPath, "daemon");
4445
pb.redirectErrorStream(false);
45-
pb.environment().remove("RUST_LOG"); // don't pollute
4646

4747
process = pb.start();
4848
writer = new BufferedWriter(new OutputStreamWriter(process.getOutputStream()));
@@ -61,13 +61,21 @@ public synchronized boolean start(String binaryPath) {
6161
}
6262

6363
public synchronized void stop() {
64-
running = false;
6564
try {
6665
if (writer != null) {
6766
sendShutdown();
68-
writer.close();
67+
writer.flush();
6968
}
7069
} catch (IOException ignored) {}
70+
running = false;
71+
try {
72+
if (process != null) process.waitFor(1, TimeUnit.SECONDS);
73+
} catch (InterruptedException ignored) {
74+
Thread.currentThread().interrupt();
75+
}
76+
try {
77+
if (writer != null) writer.close();
78+
} catch (IOException ignored) {}
7179
if (process != null) process.destroyForcibly();
7280
if (readerThread != null) readerThread.interrupt();
7381
process = null;

eclipse-plugin/src/main/java/xyz/devglobe/eclipse/core/CoreDownloader.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ public static String detectPlatform() {
4141
platform = arch.contains("aarch64") || arch.contains("arm")
4242
? "linux-arm64" : "linux-x64";
4343
} else if (os.contains("win")) {
44-
platform = arch.contains("aarch64") || arch.contains("arm")
45-
? "win-arm64" : "win-x64";
44+
// Only win-x64 is published; it runs under emulation on Windows ARM.
45+
platform = "win-x64";
4646
} else {
4747
platform = "linux-x64"; // fallback
4848
}

eclipse-plugin/src/main/java/xyz/devglobe/eclipse/core/DevGlobeTracker.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public static DevGlobeTracker getInstance() {
3030
private final List<Runnable> stateListeners = new ArrayList<>();
3131
private final AtomicBoolean starting = new AtomicBoolean(false);
3232
private final AtomicBoolean intentionalShutdown = new AtomicBoolean(false);
33-
private CoreClient client;
33+
private volatile CoreClient client;
3434

3535
// ── Public API ───────────────────────────────────────────────────────
3636

@@ -74,7 +74,11 @@ public void start() {
7474
starting.set(false);
7575
return;
7676
}
77-
ensureCore();
77+
// ensureCore() may download the core binary (blocking HTTP), so it must
78+
// never run on the UI thread.
79+
Thread worker = new Thread(this::ensureCore, "DevGlobe-Start");
80+
worker.setDaemon(true);
81+
worker.start();
7882
}
7983

8084
public void pause() {

eclipse-plugin/src/main/java/xyz/devglobe/eclipse/core/Notifier.java

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ public static void error(String message) {
4646
}
4747

4848
/**
49-
* Show a confirmation notification that the user is guaranteed to see.
50-
* Uses the status line if available, but always falls back to a dialog
51-
* if the status line cannot be reached.
49+
* Show a confirmation notification in the status line. Routine confirmations
50+
* (paused, resumed, reconnected, status updated) should not interrupt with a
51+
* modal dialog — only errors do.
5252
*/
5353
public static void confirm(String message) {
54-
show(Severity.INFO, message, true);
54+
show(Severity.INFO, message, false);
5555
}
5656

5757
// ── Implementation ───────────────────────────────────────────────────
@@ -195,25 +195,14 @@ private static StatusLineManager getStatusLineManager(IWorkbenchWindow window) {
195195
}
196196
}
197197

198-
/** Clear the status-line message after a short delay. */
198+
/** Clear the status-line message after a short delay, on the UI thread. */
199199
private static void scheduleClear(Display display, StatusLineManager slm) {
200-
Thread clearer = new Thread(() -> {
200+
if (display == null || display.isDisposed()) return;
201+
display.timerExec(5_000, () -> {
201202
try {
202-
Thread.sleep(5_000);
203-
} catch (InterruptedException ignored) {}
204-
// Check if display is still valid before using it
205-
Display currentDisplay = PlatformUI.isWorkbenchRunning() ? PlatformUI.getWorkbench().getDisplay() : null;
206-
if (currentDisplay == null || currentDisplay.isDisposed()) {
207-
return;
208-
}
209-
currentDisplay.asyncExec(() -> {
210-
try {
211-
slm.setMessage(null);
212-
slm.setErrorMessage(null);
213-
} catch (Exception ignored) {}
214-
});
215-
}, "DevGlobe-Notifier-Clear");
216-
clearer.setDaemon(true);
217-
clearer.start();
203+
slm.setMessage(null);
204+
slm.setErrorMessage(null);
205+
} catch (Exception ignored) {}
206+
});
218207
}
219208
}

eclipse-plugin/src/main/java/xyz/devglobe/eclipse/ui/DevGlobeStatusBarContribution.java

Lines changed: 0 additions & 59 deletions
This file was deleted.

0 commit comments

Comments
 (0)