Skip to content

Commit b88b62d

Browse files
aledbfclaude
andcommitted
fix(cli): print a null devPort the loader left at the top level
read-configuration dropped a null devPort from the configuration it prints, written for a loader that did not move a null devPort into customizations.vscode. The loader now does, as TS does, so a null devPort stays at the top level only when customizations.vscode has its own, and TS prints it there. The host phase's containerWorkspaceFolder resolution becomes two conditions, so each one is held by a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 6c2a595 commit b88b62d

3 files changed

Lines changed: 23 additions & 13 deletions

File tree

‎internal/cli/read_configuration.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,11 @@ func loadReadConfiguration(workspaceFolder, configPath, overridePath string, opt
258258
// 0.88: read-configuration exits 1 silently (banner only) when no
259259
// config is found — it does not print an error envelope. Parse/other
260260
// errors still surface normally.
261-
if _, ok := errors.AsType[*config.NotFoundError](err); ok {
261+
// TS read-configuration exits 1 the same way on a config that is not a
262+
// JSON object.
263+
_, notFound := errors.AsType[*config.NotFoundError](err)
264+
_, notObject := errors.AsType[*config.NotObjectError](err)
265+
if notFound || notObject {
262266
return nil, &coreerrors.ExitCodeError{Code: 1}
263267
}
264268
return nil, err
@@ -292,11 +296,6 @@ func configurationOutput(result *config.LoadResult, opts *readConfigOpts) map[st
292296
if _, known := cfgMap[k]; known {
293297
continue
294298
}
295-
// TS moves a null devPort into customizations.vscode, which the loader
296-
// does not; it is never printed at the top level.
297-
if k == "devPort" && v == nil {
298-
continue
299-
}
300299
cfgMap[k] = v
301300
}
302301
scheme := "vscode-fileHost"

‎internal/cli/read_configuration_nodocker_held_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import (
1111
// A legacy property TS updateFromOldProperties leaves in place stays in the
1212
// configuration read-configuration prints: a devPort whose
1313
// customizations.vscode already has one, and a falsy settings. The TS CLI
14-
// prints both (reference read-configuration of this config); a null devPort,
15-
// which TS moves into customizations.vscode, is never printed at the top level.
14+
// prints both (reference read-configuration of this config). A null devPort
15+
// moves into customizations.vscode when that has none, and otherwise stays at
16+
// the top level and is printed as null, as TS prints it.
1617
func TestConfigurationOutputKeepsUnmigratedLegacyProperties(t *testing.T) {
1718
_, result := loadFixtureConfig(t, `{
1819
"image": "img:1",
@@ -36,6 +37,12 @@ func TestConfigurationOutputKeepsUnmigratedLegacyProperties(t *testing.T) {
3637
if v, ok := got["x-null"]; !ok || v != nil {
3738
t.Errorf("x-null = %v (%v), want an unknown null property kept", v, ok)
3839
}
40+
41+
_, result = loadFixtureConfig(t, `{"image": "img:1", "devPort": null, "customizations": {"vscode": {"devPort": 1}}}`, nil)
42+
got = configurationOutput(result, &readConfigOpts{})
43+
if v, ok := got["devPort"]; !ok || v != nil {
44+
t.Errorf("devPort = %v (%v), want null kept beside customizations.vscode's own", v, ok)
45+
}
3946
}
4047

4148
// The config's forwardPorts are part of its metadata entry, a single one too.

‎internal/config/varsub.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,16 @@ type VariableResolver struct{}
3434
func NewVariableResolver() *VariableResolver { return &VariableResolver{} }
3535

3636
func (r *VariableResolver) Resolve(ctx SubstitutionContext, phase SubstitutionPhase, value any) (any, error) {
37-
if phase == PhaseHost && ctx.ContainerWorkspaceFolder != "" {
38-
resolved, err := r.resolveString(ctx, phase, ctx.ContainerWorkspaceFolder)
39-
if err != nil {
40-
return nil, err
37+
// TS substitute resolves the container workspace folder before the value
38+
// that may name it. Only the host phase reads it (resolveHostTag).
39+
if phase == PhaseHost {
40+
if ctx.ContainerWorkspaceFolder != "" {
41+
resolved, err := r.resolveString(ctx, phase, ctx.ContainerWorkspaceFolder)
42+
if err != nil {
43+
return nil, err
44+
}
45+
ctx.ContainerWorkspaceFolder = resolved
4146
}
42-
ctx.ContainerWorkspaceFolder = resolved
4347
}
4448
return r.resolveRecursive(ctx, phase, value)
4549
}

0 commit comments

Comments
 (0)