Suggested changes.#144
Conversation
e997eba to
3322218
Compare
3322218 to
6156955
Compare
a3c2df4 to
975b7c3
Compare
975b7c3 to
735801f
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends InjectionNext’s client/server command protocol to support pushing configuration via environment variables at runtime, and updates the macOS app UI/behavior around Xcode launching, status display, and settings tooltips.
Changes:
- Add a new
InjectionSetenvcommand and implement handling tosetenv/unsetenvin the Swift package client. - Send selected config values to connected clients (on connect and when settings change).
- UI/UX tweaks across Settings + status menu (tooltips, labels), plus some Xcode monitoring and minor safety tweaks.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/InjectionNextC/include/SimpleSocket.h | Exposes fdopenForMode: on SimpleSocket for FILE stream creation. |
| Sources/InjectionNextC/include/InjectionClient.h | Adds InjectionSetenv to the shared command enum. |
| Sources/InjectionNextC/SimpleSocket.mm | Implements fdopenForMode:. |
| Sources/InjectionNext/InjectionNext.swift | Handles new .setenv command and improves unknown-command error text. |
| README.md | Clarifies linker flag is for Debug. |
| App/feedcommands/main.mm | Switches to fdopenForMode: helper for stream creation. |
| App/InjectionNext/Views/XcodeSettingsView.swift | Adds tooltips and updates “Xcode running” status presentation. |
| App/InjectionNext/Views/StatusMenuView.swift | Reorders menu items, adds tooltips, adjusts actions, removes selected-project UI. |
| App/InjectionNext/Views/InjectionSettingsView.swift | Removes the “Project Path” selection section from this view. |
| App/InjectionNext/Views/GeneralSettingsView.swift | Adds tooltips and switches status fields to “Launched Xcode”. |
| App/InjectionNext/Views/FileWatcherSettingsView.swift | Adds tooltips for pattern/latency controls. |
| App/InjectionNext/Views/DevicesSettingsView.swift | Adds tooltips and renames “Device Testing” section. |
| App/InjectionNext/Views/CompilerSettingsView.swift | Adds tooltip to compiler settings form. |
| App/InjectionNext/Views/BuildSystemSettingsView.swift | Adds tooltips to Bazel/xcrun controls. |
| App/InjectionNext/Views/AdvancedSettingsView.swift | Adds tooltip for verbose logging section. |
| App/InjectionNext/MonitorXcode.swift | Tracks “launched Xcode” state, blocks launching if Xcode already running, updates save hook. |
| App/InjectionNext/InjectionServer.swift | Sends env vars to newly connected clients. |
| App/InjectionNext/InjectionHybrid.swift | Uses configurable injectable regex; adjusts hybrid compiler behavior. |
| App/InjectionNext/Info.plist | Bumps build number. |
| App/InjectionNext/FrontendServer.swift | Makes patchCompilerItem update nil-safe. |
| App/InjectionNext/ConfigStore.swift | Adds env var propagation on setting changes; switches to “haveLaunchedXocde” flag; thread-handling tweak. |
| App/InjectionNext/AppDelegate.swift | Starts FrontendServer earlier when patched; changes log capture behavior; updates connection/UI state bridging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Button { | ||
| selectProject() | ||
| AppDelegate.ui?.runXcode(self) | ||
| } label: { |
There was a problem hiding this comment.
This change removes the only call site for selectProject() in this view, leaving the selectProject() helper unused (and potentially producing an "unused private function" warning depending on build settings). Either remove the dead code or reintroduce a UI action that uses it.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
469ace0 to
d7282fa
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
642a800 to
ef32835
Compare
ef32835 to
9295705
Compare
Adds seven concrete backlog items under three new subsections in TODO.md's Next list. All discovered while running a maatheusgois-dd fork of 2.0_overhaul against real iOS / Bazel codebases. Hybrid / file-watcher - Allow file-watcher injection while Xcode is running (drop the MonitorXcode.runningXcode early-return in InjectionHybrid). - Skip SDK filter in log-parsing Recompiler when no client is connected (avoids grep SDKs/MacOSX misses for iOS projects edited before first device/sim connect). - Replace DispatchQueue.main.sync on pendingFilesChanged with an NSLock to avoid main-queue deadlocks during compile I/O. Developer tooling - Reuse already-running Xcode in AppDelegate.runXcode instead of Popen-launching a duplicate Xcode process (macOS single-instances Xcode.app, leaving runningXcode pointed at a dead pipe). - Top-level Makefile for local dev. - CI release workflow (.github/workflows/release.yml). Integrations / observability - Injection event tracker emitting structured events over the existing ControlServer for MCP / AI agent consumers. No source changes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1cc5159 to
765220e
Compare
4027400 to
01694a5
Compare
01694a5 to
1028341
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| detail("SwiftTracing \(name)") | ||
| return autoBitCast(SwiftTrace.trace(name: name, original: injected)) ?? injected | ||
| + #"|InjectionBundle.|fast_dl"# | ||
| for name in [INJECTION_TRACE_LOOKUP, INJECTION_TRACE_LOOKUP, INJECTION_TRACE_FILTER, |
There was a problem hiding this comment.
tracingOptions() iterates over a list of env var names, but INJECTION_TRACE_LOOKUP appears twice. This is redundant and makes it easier to miss a variable that should be included.
| for name in [INJECTION_TRACE_LOOKUP, INJECTION_TRACE_LOOKUP, INJECTION_TRACE_FILTER, | |
| for name in [INJECTION_TRACE_LOOKUP, INJECTION_TRACE_FILTER, |
| LabeledContent("Launched Xcode") { | ||
| HStack { | ||
| Circle() | ||
| .fill(config.isXcodeRunning ? .green : .gray) | ||
| .fill(config.haveLaunchedXcode ? .green : .gray) | ||
| .frame(width: 8, height: 8) | ||
| Text(config.isXcodeRunning ? "Running" : "Not Running") | ||
| Text(config.haveLaunchedXcode ? "Running" : "Not Running") | ||
| } | ||
| } | ||
| .help("Was Xcode launched by this app") | ||
| Button("Launch Xcode Now") { | ||
| AppDelegate.ui?.runXcode(self) | ||
| } | ||
| .disabled(config.isXcodeRunning) | ||
| .disabled(config.haveLaunchedXcode) | ||
| .help("Launch Xcode if it isn’t already running") |
There was a problem hiding this comment.
The UI is using haveLaunchedXcode to display “Running/Not Running” and to disable the “Launch Xcode Now” button. haveLaunchedXcode is updated from MonitorXcode.runningXcode (i.e., only when Xcode was launched/monitored by this app), so if Xcode is already running externally MonitorXcode() will fail but this view will still show “Not Running” and keep the button enabled. Consider either (a) reflecting external Xcode state (e.g. MonitorXcode.externalXcode != nil) for the running indicator/disabled state, or (b) changing the text to avoid implying a process-running status.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
OK, after a flurry of last minute changes implementing the tracing options I've merged this PR and prepared a release candidate 2.0.0RC1. Over to you! You have control. |
Hi @maatheusgois-dd, These are some changes I'd like to make to the new code.