Skip to content

Commit 1efd547

Browse files
AshotNAshotN
committed
refactor: use pending state to decouple UI from persisted settings in OpenCodeSettingsConfigurable
1 parent ba3acee commit 1efd547

2 files changed

Lines changed: 138 additions & 82 deletions

File tree

‎src/main/kotlin/com/ashotn/opencode/companion/settings/OpenCodeSettingsConfigurable.kt‎

Lines changed: 87 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,21 @@ import java.util.concurrent.atomic.AtomicReference
2323
class OpenCodeSettingsConfigurable(private val project: Project) :
2424
BoundConfigurable("OpenCode Companion") {
2525

26+
private val pendingState = OpenCodeSettings.State()
27+
2628
internal var executableResolver: (String?) -> OpenCodeInfo? = { path -> OpenCodeChecker.findExecutable(path) }
2729

2830
override fun createPanel(): DialogPanel {
29-
val settings = OpenCodeSettings.getInstance(project)
31+
loadPendingFromPersisted()
32+
3033
return panel {
3134
group("Executable") {
3235
val executablePathField = TextFieldWithBrowseButton().apply {
3336
addBrowseFolderListener(
3437
project,
3538
FileChooserDescriptorFactory.createSingleFileNoJarsDescriptor()
3639
.withTitle("Select OpenCode Executable")
37-
.withDescription("Choose the opencode executable file.")
40+
.withDescription("Choose the opencode executable file."),
3841
)
3942
}
4043
run {
@@ -57,33 +60,33 @@ class OpenCodeSettingsConfigurable(private val project: Project) :
5760
}
5861
row("OpenCode Path:") {
5962
cell(executablePathField)
60-
.bindText(settings::executablePath)
63+
.bindText(pendingState::executablePath)
6164
.comment("Path to the opencode executable. Leave blank to auto-detect.")
6265
.align(AlignX.FILL)
6366
}
6467
}
6568
group("Server") {
6669
row("Server Port:") {
6770
intTextField(1024..65535)
68-
.bindIntText(settings::serverPort)
71+
.bindIntText(pendingState::serverPort)
6972
.comment("Port the OpenCode server listens on (default: 4096)")
7073
}
7174
}
7275
group("Editor") {
7376
row {
7477
checkBox("Show inline diff highlights")
75-
.bindSelected(settings::inlineDiffEnabled)
78+
.bindSelected(pendingState::inlineDiffEnabled)
7679
.comment(
7780
"Renders green/red inline diff highlights in the editor " +
78-
"for AI-modified files. Changes take effect immediately."
81+
"for AI-modified files. Changes take effect immediately.",
7982
)
8083
}
8184
}
8285
group("Terminal") {
8386
val reworkedSupported = BuildUtils.isEmbeddedTerminalSupported
8487
row {
8588
checkBox("Show inline terminal")
86-
.bindSelected(settings::inlineTerminalEnabled)
89+
.bindSelected(pendingState::inlineTerminalEnabled)
8790
.comment("Embeds the OpenCode TUI directly inside the tool window panel when the server is running.")
8891
}
8992
buttonsGroup("Terminal engine:") {
@@ -96,138 +99,141 @@ class OpenCodeSettingsConfigurable(private val project: Project) :
9699
.enabled(reworkedSupported)
97100
.comment(
98101
if (reworkedSupported) "New terminal engine (IntelliJ 2025.3+)."
99-
else "Requires IntelliJ 2025.3 or later."
102+
else "Requires IntelliJ 2025.3 or later.",
100103
)
101104
}
102-
}.bind(settings::terminalEngine)
105+
}.bind(pendingState::terminalEngine)
103106
}
104107
group("Diagnostics") {
105108
row {
106109
checkBox("Enable diff trace logging")
107-
.bindSelected(settings::diffTraceEnabled)
110+
.bindSelected(pendingState::diffTraceEnabled)
108111
.comment(
109112
"Writes a JSONL trace file to the system temp directory " +
110113
"(opencode-diff-traces/) for debugging diff pipeline events. " +
111-
"Takes effect after restarting the IDE."
114+
"Takes effect after restarting the IDE.",
112115
)
113116
}
114117
row {
115118
checkBox("Include historical diffs in trace")
116-
.bindSelected(settings::diffTraceHistoryEnabled)
119+
.bindSelected(pendingState::diffTraceHistoryEnabled)
117120
.comment(
118121
"Also records events from historical (loaded-on-demand) session diffs " +
119122
"in the trace. Only relevant when diff trace logging is enabled. " +
120-
"Takes effect after restarting the IDE."
123+
"Takes effect after restarting the IDE.",
121124
)
122125
}
123126
}
124127
}
125128
}
126129

130+
override fun reset() {
131+
loadPendingFromPersisted()
132+
super.reset()
133+
}
134+
127135
override fun apply() {
128136
val settings = OpenCodeSettings.getInstance(project)
129137
val plugin = OpenCodePlugin.getInstance(project)
130-
val oldSettings = snapshot(settings)
138+
val oldSettings = snapshot(settings.state)
131139
val oldOpenCodeInfo = plugin.openCodeInfo
132140

133-
super.apply()
141+
super.apply() // Pushes UI values into pendingState.
134142

135-
val newSettings = snapshot(settings)
143+
val newSettings = snapshot(pendingState)
144+
val settingsChanged = newSettings != oldSettings
136145
val newPort = newSettings.serverPort
137146
val newPath = newSettings.executablePath
138147
val portChanged = newPort != oldSettings.serverPort
139148
val pathChanged = newPath != oldSettings.executablePath
140149
val shouldResolveInfo = pathChanged || (newPath.isBlank() && oldOpenCodeInfo == null)
141-
val portOrPathChanged = portChanged || pathChanged
142-
val serverRunning = plugin.isRunning
143-
val owned = plugin.ownsProcess
144-
val mustConfirmStop = serverRunning && owned && portOrPathChanged
145-
val mustReattach = serverRunning && !owned && portChanged
150+
if (!settingsChanged && !shouldResolveInfo) return
146151

147-
var resolvedInfo: OpenCodeInfo? = oldOpenCodeInfo
148-
var openCodeInfoResolved = false
152+
val mustConfirmStop = plugin.isRunning && plugin.ownsProcess && (portChanged || pathChanged)
153+
val mustReattach = plugin.isRunning && !plugin.ownsProcess && portChanged
149154

155+
var resolvedInfo = oldOpenCodeInfo
150156
if (shouldResolveInfo) {
151157
val userProvidedPath = newPath.takeIf { it.isNotBlank() }
152-
val resolvedRef = AtomicReference<OpenCodeInfo?>()
153-
ProgressManager.getInstance().runProcessWithProgressSynchronously(
154-
{
155-
resolvedRef.set(executableResolver(userProvidedPath))
156-
},
157-
"Resolving OpenCode\u2026",
158-
false,
159-
project
160-
)
161-
162-
resolvedInfo = resolvedRef.get()
163-
if (isExecutableResolutionFailureBlocking(userProvidedPath, resolvedInfo)) {
164-
restore(settings, oldSettings)
158+
resolvedInfo = resolveExecutableInfo(userProvidedPath)
159+
if (userProvidedPath != null && resolvedInfo == null) {
165160
throw ConfigurationException(
166-
"Could not find a valid OpenCode executable. Check the path and try again."
161+
"Could not find a valid OpenCode executable. Check the path and try again.",
167162
)
168163
}
169-
170-
openCodeInfoResolved = true
171164
}
172165

173-
if (mustConfirmStop) {
174-
val confirmed = Messages.showYesNoDialog(
175-
project,
176-
"The OpenCode server is currently running. Applying these changes will stop it. " +
177-
"You will need to start it again manually.",
178-
"Stop OpenCode Server?",
179-
"Stop Server",
180-
"Cancel",
181-
Messages.getWarningIcon()
182-
) == Messages.YES
183-
184-
if (!confirmed) {
185-
restore(settings, oldSettings)
186-
reset()
187-
return
188-
}
166+
if (mustConfirmStop && !confirmStopServerRestart()) {
167+
reset()
168+
return
169+
}
189170

190-
plugin.stopServer()
171+
persistPendingToSettings(settings)
191172

192-
} else if (mustReattach) {
193-
plugin.reattach(newPort)
173+
when {
174+
mustConfirmStop -> plugin.stopServer()
175+
mustReattach -> plugin.reattach(newPort)
194176
}
195177

196-
if (openCodeInfoResolved && resolvedInfo != oldOpenCodeInfo) {
178+
if (shouldResolveInfo && resolvedInfo != oldOpenCodeInfo) {
197179
plugin.openCodeInfo = resolvedInfo
198180
project.messageBus.syncPublisher(OpenCodeInfoChangedListener.TOPIC)
199181
.onOpenCodeInfoChanged(resolvedInfo)
200182
}
201183

202-
val finalSettings = snapshot(settings)
203-
if (finalSettings != oldSettings) {
184+
if (settingsChanged) {
204185
project.messageBus.syncPublisher(OpenCodeSettingsChangedListener.TOPIC)
205-
.onSettingsChanged(oldSettings, finalSettings)
186+
.onSettingsChanged(oldSettings, newSettings)
206187
}
207188

208189
EditorDiffRenderer.getInstance(project).onSettingsChanged()
209190
}
210191

211-
private fun restore(settings: OpenCodeSettings, snapshot: OpenCodeSettingsSnapshot) {
212-
settings.serverPort = snapshot.serverPort
213-
settings.executablePath = snapshot.executablePath
214-
settings.inlineDiffEnabled = snapshot.inlineDiffEnabled
215-
settings.diffTraceEnabled = snapshot.diffTraceEnabled
216-
settings.diffTraceHistoryEnabled = snapshot.diffTraceHistoryEnabled
217-
settings.inlineTerminalEnabled = snapshot.inlineTerminalEnabled
218-
settings.terminalEngine = snapshot.terminalEngine
192+
private fun resolveExecutableInfo(userProvidedPath: String?): OpenCodeInfo? {
193+
val resolvedRef = AtomicReference<OpenCodeInfo?>()
194+
ProgressManager.getInstance().runProcessWithProgressSynchronously(
195+
{
196+
resolvedRef.set(executableResolver(userProvidedPath))
197+
},
198+
"Resolving OpenCode...",
199+
false,
200+
project,
201+
)
202+
return resolvedRef.get()
203+
}
204+
205+
private fun confirmStopServerRestart(): Boolean =
206+
Messages.showYesNoDialog(
207+
project,
208+
"The OpenCode server is currently running. Applying these changes will stop it. " +
209+
"You will need to start it again manually.",
210+
"Stop OpenCode Server?",
211+
"Stop Server",
212+
"Cancel",
213+
Messages.getWarningIcon(),
214+
) == Messages.YES
215+
216+
private fun loadPendingFromPersisted(settings: OpenCodeSettings = OpenCodeSettings.getInstance(project)) {
217+
pendingState.serverPort = settings.serverPort
218+
pendingState.executablePath = settings.executablePath
219+
pendingState.inlineDiffEnabled = settings.inlineDiffEnabled
220+
pendingState.diffTraceEnabled = settings.diffTraceEnabled
221+
pendingState.diffTraceHistoryEnabled = settings.diffTraceHistoryEnabled
222+
pendingState.inlineTerminalEnabled = settings.inlineTerminalEnabled
223+
pendingState.terminalEngine = settings.terminalEngine
219224
}
220225

221-
private fun snapshot(settings: OpenCodeSettings): OpenCodeSettingsSnapshot = OpenCodeSettingsSnapshot(
222-
serverPort = settings.serverPort,
223-
executablePath = settings.executablePath,
224-
inlineDiffEnabled = settings.inlineDiffEnabled,
225-
diffTraceEnabled = settings.diffTraceEnabled,
226-
diffTraceHistoryEnabled = settings.diffTraceHistoryEnabled,
227-
inlineTerminalEnabled = settings.inlineTerminalEnabled,
228-
terminalEngine = settings.terminalEngine,
226+
private fun persistPendingToSettings(settings: OpenCodeSettings) {
227+
settings.loadState(pendingState.copy())
228+
}
229+
230+
private fun snapshot(state: OpenCodeSettings.State): OpenCodeSettingsSnapshot = OpenCodeSettingsSnapshot(
231+
serverPort = state.serverPort,
232+
executablePath = state.executablePath,
233+
inlineDiffEnabled = state.inlineDiffEnabled,
234+
diffTraceEnabled = state.diffTraceEnabled,
235+
diffTraceHistoryEnabled = state.diffTraceHistoryEnabled,
236+
inlineTerminalEnabled = state.inlineTerminalEnabled,
237+
terminalEngine = state.terminalEngine,
229238
)
230239
}
231-
232-
internal fun isExecutableResolutionFailureBlocking(userProvidedPath: String?, resolvedInfo: OpenCodeInfo?): Boolean =
233-
userProvidedPath != null && resolvedInfo == null

‎src/test/kotlin/com/ashotn/opencode/companion/settings/OpenCodeSettingsConfigurableTest.kt‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import com.intellij.openapi.ui.TextFieldWithBrowseButton
88
import com.intellij.testFramework.fixtures.BasePlatformTestCase
99
import java.awt.Component
1010
import java.awt.Container
11+
import java.util.concurrent.atomic.AtomicInteger
1112
import java.util.concurrent.atomic.AtomicReference
1213
import kotlin.test.assertFailsWith
1314

@@ -31,14 +32,15 @@ class OpenCodeSettingsConfigurableTest : BasePlatformTestCase() {
3132
runOnEdt { configurable.apply() }
3233

3334
assertEquals("", settings.executablePath)
35+
assertNull(OpenCodePlugin.getInstance(project).openCodeInfo)
3436
} finally {
3537
runOnEdt { configurable.disposeUIResources() }
3638
}
3739
}
3840

3941
fun testApplyBlocksSavingWhenExplicitPathFailsResolution() {
4042
val settings = OpenCodeSettings.getInstance(project)
41-
settings.executablePath = ""
43+
settings.executablePath = "C:/existing/opencode"
4244

4345
val configurable = OpenCodeSettingsConfigurable(project).apply {
4446
executableResolver = { null }
@@ -54,12 +56,60 @@ class OpenCodeSettingsConfigurableTest : BasePlatformTestCase() {
5456
assertFailsWith<ConfigurationException> {
5557
runOnEdt { configurable.apply() }
5658
}
59+
assertEquals("C:/existing/opencode", settings.executablePath)
60+
} finally {
61+
runOnEdt { configurable.disposeUIResources() }
62+
}
63+
}
64+
65+
fun testApplyAttemptsAutoResolveWhenPathBlankAndInfoMissing() {
66+
val settings = OpenCodeSettings.getInstance(project)
67+
settings.executablePath = ""
68+
OpenCodePlugin.getInstance(project).openCodeInfo = null
69+
70+
val resolveCalls = AtomicInteger(0)
71+
val configurable = OpenCodeSettingsConfigurable(project).apply {
72+
executableResolver = {
73+
resolveCalls.incrementAndGet()
74+
null
75+
}
76+
}
77+
78+
try {
79+
getOnEdt { configurable.createComponent() }
80+
runOnEdt { configurable.apply() }
81+
82+
assertEquals(1, resolveCalls.get())
5783
assertEquals("", settings.executablePath)
5884
} finally {
5985
runOnEdt { configurable.disposeUIResources() }
6086
}
6187
}
6288

89+
fun testApplySkipsResolveWhenNoSettingsChangeAndInfoAlreadyResolved() {
90+
val settings = OpenCodeSettings.getInstance(project)
91+
settings.executablePath = "C:/existing/opencode"
92+
OpenCodePlugin.getInstance(project).openCodeInfo = OpenCodeInfo(settings.executablePath, "1.2.3")
93+
94+
val resolveCalls = AtomicInteger(0)
95+
val configurable = OpenCodeSettingsConfigurable(project).apply {
96+
executableResolver = {
97+
resolveCalls.incrementAndGet()
98+
OpenCodeInfo("C:/resolved/opencode", "1.2.3")
99+
}
100+
}
101+
102+
try {
103+
getOnEdt { configurable.createComponent() }
104+
runOnEdt { configurable.apply() }
105+
106+
assertEquals(0, resolveCalls.get())
107+
assertEquals("C:/existing/opencode", settings.executablePath)
108+
} finally {
109+
runOnEdt { configurable.disposeUIResources() }
110+
}
111+
}
112+
63113
private fun runOnEdt(action: () -> Unit) {
64114
ApplicationManager.getApplication().invokeAndWait(action)
65115
}

0 commit comments

Comments
 (0)