fix: restore alphabetical sorting for sde generate --preprocess com…

…mand (#587) Fixes #586
climateinteractive · Dec 8, 2024 · 039fb79 · 039fb79
1 parent 25412f2
commit 039fb79
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 28 deletions.
diff --git a/models/comments/expected.mdl b/models/comments/expected.mdl
@@ -1,7 +1,5 @@
 {UTF-8}
 
-DimA: A1, A2, A3 ~~|
-
 a[DimA]= 0, 1, 2 ~~|
 
 b = 3 ~~|
@@ -10,12 +8,14 @@ c = 4 ~~|
 
 d= 8760 ~~|
 
-e = 41 + 1 ~~|
+DimA: A1, A2, A3 ~~|
 
-INITIAL TIME = 0 ~~|
+e = 41 + 1 ~~|
 
 FINAL TIME = 1 ~~|
 
+INITIAL TIME = 0 ~~|
+
 SAVEPER = 1 ~~|
 
 TIME STEP = 1 ~~|
diff --git a/models/preprocess/expected.mdl b/models/preprocess/expected.mdl
@@ -1,42 +1,42 @@
 {UTF-8}
 
-" quotes 2 with extra whitespace " = 1 ~~|
-
-"Quotes 1" = 1 ~~|
+Apple 1 = 1 ~~|
 
-"quotes 3 with | pipe character " = 2 ~~|
+Apple 2 = 1 ~~|
 
 apple 3 = 1 ~~|
 
-Carrot 3 Data 2 ~~|
+Banana 1 = 1 ~~|
+
+Banana 2 = 1 ~~|
 
 banana 3 = Banana 1 * Banana 2 ~~|
 
-carrot 3 Data 1 ~~|
+carrot 1 = 1 ~~|
 
 carrot 2 = 1 ~~|
 
-Look2((0,0),(1,1),(2,2)) ~~|
+carrot 3 Data 1 ~~|
 
-Look1((0,0),(1,1),(2,2)) ~~|
+Carrot 3 Data 2 ~~|
 
-Apple 2 = 1 ~~|
+DimA: A1, A2 ~~|
 
-carrot 1 = 1 ~~|
+dimB: B1, B2 ~~|
 
-Banana 2 = 1 ~~|
+FINAL TIME = 10 ~~|
 
-Banana 1 = 1 ~~|
+INITIAL TIME = 0 ~~|
 
-Apple 1 = 1 ~~|
+Look1((0,0),(1,1),(2,2)) ~~|
 
-dimB: B1, B2 ~~|
+Look2((0,0),(1,1),(2,2)) ~~|
 
-DimA: A1, A2 ~~|
+"Quotes 1" = 1 ~~|
 
-FINAL TIME = 10 ~~|
+" quotes 2 with extra whitespace " = 1 ~~|
 
-INITIAL TIME = 0 ~~|
+"quotes 3 with | pipe character " = 2 ~~|
 
 SAVEPER = TIME STEP ~~|
 

diff --git a/packages/cli/src/sde-generate.js b/packages/cli/src/sde-generate.js
@@ -108,6 +108,13 @@ function preprocessModel(mdlContent) {
   // Run the preprocessor
   const { defs } = preprocessVensimModel(mdlContent)
 
+  // Sort the definitions alphabetically by key.  This mainly exists for compatibility
+  // with the legacy `sde generate --preprocess` command, which was changed in #55 to
+  // sort definitions alphabetically.
+  defs.sort((a, b) => {
+    return a.key < b.key ? -1 : a.key > b.key ? 1 : 0
+  })
+
   // Join the preprocessed definitions into a single string
   let text = '{UTF-8}\n'
   for (const def of defs) {

diff --git a/packages/compile/src/parse-and-generate.js b/packages/compile/src/parse-and-generate.js
@@ -157,11 +157,6 @@ export function parseModel(input, modelDir, options) {
   }
 
   // Parse the model
-  // TODO: The `parseVensimModel` function implicitly runs the preprocess step
-  // on the input text.  We currently allow for setting the `sort` flag only
-  // for use in tests that assume the behavior of the legacy preprocessor
-  // (because it sorted alphabetically).  Once we update those tests, we can
-  // remove this option.
   const sort = options?.sort === true
   const root = parseVensimModel(input, parseContext, sort)
 

diff --git a/packages/parse/src/vensim/parse-vensim-model.ts b/packages/parse/src/vensim/parse-vensim-model.ts
@@ -22,8 +22,9 @@ export function parseVensimModel(input: string, context?: VensimParseContext, so
   // sections, etc) so that it can be parsed more easily by `antlr4-vensim`.
   const { defs } = preprocessVensimModel(input)
   if (sort) {
-    // XXX: This sorting is currently only needed for compatibility with the legacy
-    // preprocessor, which sorted definitions alphabetically.  Can consider removing this.
+    // Sort the definitions alphabetically by key.  This mainly exists for compatibility
+    // with the legacy preprocessor; it is not an essential step so is left disabled
+    // by default.
     defs.sort((a, b) => {
       return a.key < b.key ? -1 : a.key > b.key ? 1 : 0
     })