Repository navigation
fix(cli): keep JS property order in configuration output and docker run - #77
Merged
Merged
Conversation
read-configuration, up and set-up wrote configuration's containerEnv, features and each Feature's options, and mergedConfiguration's containerEnv, with sorted keys. TS writes them as JSON.parse and mergeConfiguration built them: integer-like keys ascending, then the others in declaration order. up passed the merged containerEnv to docker run as -e flags in Go map order, which changed between runs; TS passes them in Object.keys order. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What TS does
configurationinread-configuration,upandset-upis the devcontainer.json object asJSON.parsebuilt it. ItscontainerEnv,featuresand each Feature's options keep JS property order: integer-like keys first, ascending, then the others as declared.mergedConfiguration.containerEnvismergeConfiguration'sObject.assignover the metadata entries, so a key keeps the position it first had.uppasses the merged containerEnv todocker runas-eflags inObject.keysorder (singleContainer.ts).Before / after
Input:
containerEnv{ZED, 10, ALPHA, 2},features{zeta: {zz, 10, a, 2}, 10, alpha}.configuration.containerEnv10, 2, ALPHA, ZED2, 10, ZED, ALPHAconfiguration.features10, alpha, zeta10, zeta, alphaconfiguration.features.zeta10, 2, a, zz2, 10, zz, amergedConfiguration.containerEnv10, 2, ALPHA, ZED2, 10, ZED, ALPHAdocker run -einup2, 10, ZED, ALPHAThis reuses the orders the loader already records (
ContainerEnvOrder,FeatureOrder,FeatureOptionOrder) andMergedConfig.ContainerEnvOrderfrom #75. The--ca-bundlevariables, which TS doesn't have, go after the configured ones.Tests
TestReadConfigurationWritesNestedObjectsInPropertyOrder. It fails on main.TestMergedConfigurationOutputreads the orderedcontainerEnvobject's fields. The assertion is unchanged./var/tmp.read-configuration.configuration-container-env-and-features-key-order-successread-configuration.merged-container-env-key-order-successup.container-env-run-order-success. It logs thedocker runargv through a--docker-pathwrapper;Config.Envisn't compared because Docker reorders it.Checks
task lint: 0 issues.BASE=origin/main task mutate: 16 refused, 0 survived.task test:race: pass, withTMPDIRoutside/tmp.task parity:contract: 116 matched. The 4 failures fail only because of a stray/tmp/.giton the dev machine; that includes the new case, which passes with a/var/tmpworkspace.task spec:compliance: pass.task test:e2e: 13/13 pass.🤖 Generated with Claude Code