Soft mode #502

frostburn · 2024-03-05T08:51:13Z

hildjj · 2025-04-08T22:02:26Z

Once #593 lands, can you rebase this, please? I'll play around with it to see if it makes sense to go into the next major release. I think there's a semver-major breaking change with existing grammars that use the custom option soft.

We will also need docs, but don't bother with those until this gets rebased and I try it out.

hildjj · 2025-04-17T20:11:07Z

ok, I'm ready for this to be rebased, please.

frostburn · 2025-04-18T18:19:15Z

Rebased. CI tests seem to pass even though locally I have three tests failing when running nve latest npm test on main and on this branch.

hildjj · 2025-04-18T20:09:56Z

Great. I will take a look at this and make a crisp decision before landing #602.

hildjj · 2025-04-18T20:40:05Z

Can you remove node_modules, do pnpm i -r and see if your local errors go away, please?

hildjj

Should there be a command line option?
lib/peg.d.ts needs to be updated, as well as the type tests in test/types/peg.test-d.ts

hildjj · 2025-04-18T20:41:34Z

lib/.eslintrc.json

This file isn't used anymore, and we should be in ES2020 on main, so this shouldn't be needed anyway, I hope.

hildjj · 2025-04-18T20:42:33Z

lib/compiler/passes/generate-js.js

-      "  if (peg$result !== peg$FAILED && peg$currPos === input.length) {",
-      "    return peg$result;",
-      "  } else {",
+      "  var success = (peg$result !== peg$FAILED && peg$currPos === input.length);",


this should be const now instead of var.

hildjj · 2025-04-18T20:45:36Z

lib/compiler/passes/generate-js.js

@@ -1373,6 +1372,14 @@ function generateJS(ast, options) {
      "        : peg$computeLocation(peg$maxFailPos, peg$maxFailPos)",
      "    );",
      "  }",
+      "  if (options.soft) {",
+      "     return {result: peg$result, success, fail};",


If success is true, maybe fail should be undefined? Calling it will always give unexpected results on success.

hildjj · 2025-04-18T20:45:51Z

test/api/generated-parser-api.spec.js

+      const parser = peg.generate("start = 'a'");
+
+      const result = parser.parse("a", { soft: true });
+      expect(result.result).to.equal("a");


Check that fail is undefined.

hildjj · 2025-04-18T20:47:58Z

Overall, I'm +1 on this, but it needs a few tweaks.

hildjj · 2025-04-19T13:41:46Z

Apologies, but I just realized that this code should build on the peg$library option. It makes sense for the results from that to include the the fail() function. That would give you everything you need, I think, except for the nicer ergonomics that your approach has. If we go that way, we could document and stabilize peg$library.

frostburn · 2025-04-21T13:59:01Z

Can you remove node_modules, do pnpm i -r and see if your local errors go away, please?

They do not. I'll chalk it off to running an old Ubuntu locally.

hildjj · 2025-04-21T14:12:21Z

Can you copy over the error you're seeing into this discussion, please?

frostburn · 2025-04-21T15:11:52Z

Changed fail() to peg$throw() and used library mode for everything.

frostburn · 2025-04-21T15:15:13Z

Can you copy over the error you're seeing into this discussion, please?

$ nve latest npm run test

> peggy@4.2.0 test
> node --experimental-vm-modules node_modules/jest/bin/jest.js

 PASS  test/unit/compiler/passes/add-imported-rules.spec.js
 PASS  test/api/generated-parser-api.spec.js
 PASS  test/unit/compiler.spec.js
 PASS  test/unit/compiler/passes/merge-character-classes.spec.js
 PASS  test/unit/compiler/passes/report-infinite-repetition.spec.js
 PASS  test/unit/compiler/passes/generate-bytecode.spec.js
 PASS  test/unit/compiler/passes/inference-match-result.spec.js
 PASS  test/api/plugin-api.spec.js
 PASS  test/unit/compiler/passes/report-infinite-recursion.spec.js
 PASS  test/cli/watcher.spec.ts
 PASS  test/unit/parser.spec.js
 PASS  test/types/peg.test-d.ts
 PASS  test/unit/compiler/passes/report-duplicate-labels.spec.js
 PASS  test/unit/compiler/passes/report-incorrect-plucking.spec.js
(node:767857) ExperimentalWarning: VM Modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
 PASS  test/unit/compiler/passes/report-duplicate-rules.spec.js
 PASS  test/unit/compiler/passes/report-undefined-rules.spec.js
 PASS  test/unit/compiler/utils.spec.js
 PASS  test/unit/grammar-error.spec.js
 PASS  test/unit/compiler/stack.spec.js
 PASS  test/cli/utils.spec.ts
 PASS  test/unit/compiler/passes/report-duplicate-imports.spec.js
 PASS  test/unit/compiler/intern.spec.js
 PASS  test/unit/compiler/passes/remove-unused-rules.spec.js
 PASS  test/api/pegjs-api.spec.js
 PASS  test/unit/compiler/passes/remove-proxy-rules.spec.js
 PASS  test/unit/compiler/passes/generate-js.spec.js
 PASS  test/unit/compiler/passes/fix-library-numbers.spec.js
 FAIL  test/cli/run.spec.ts
  ● Console

    console.log
      Delete file: "/home/lumi/JS/peggy/source.map"

      at test/cli/run.spec.ts:319:13

    console.log
      Delete "/home/lumi/JS/peggy/test/cli/output-with-default-map.js"

      at test/cli/run.spec.ts:1063:19

    console.log
      Delete file: "/home/lumi/JS/peggy/test/cli/output-with-default-map.js.map"

      at test/cli/run.spec.ts:319:13

    console.log
      Delete file: "/home/lumi/JS/peggy/test/cli/specified-name.map"

      at test/cli/run.spec.ts:319:13

  ● Command Line Interface › outputs to a file

    expect(received).toThrow()

    Received function did not throw

      814 |       // Make sure the file isn't there before we start
      815 |       fs.statSync(test_output);
    > 816 |     }).toThrow();
          |        ^
      817 |
      818 |     await exec({
      819 |       args: ["-o", test_output],

      at Object.<anonymous> (test/cli/run.spec.ts:816:8)

  ● Command Line Interface › handles source map › with default name without --output › generates a source map 1

    expect(received).toThrow()

    Received function did not throw

      318 |     fs.statSync(sourceMap);
      319 |     console.log(`Delete file: "${sourceMap}"`);
    > 320 |   }).toThrow();
          |      ^
      321 |
      322 |   await exec({
      323 |     args,

      at checkSourceMap (test/cli/run.spec.ts:320:6)
      at Object.<anonymous> (test/cli/run.spec.ts:1024:15)

  ● Command Line Interface › handles source map › with default name without --output › emits an error if used with --test/--test-file

    expect(received).toThrow()

    Received function did not throw

      1030 |           // Make sure the file isn't there before we start
      1031 |           fs.statSync(sourceMap);
    > 1032 |         }).toThrow();
           |            ^
      1033 |
      1034 |         await expect(exec({
      1035 |           args: ["-t", "1", "--source-map"],

      at Object.<anonymous> (test/cli/run.spec.ts:1032:12)

  ● Command Line Interface › handles source map › with default name with --output › generates a source map 2

    expect(received).toThrow()

    Received function did not throw

      1062 |           fs.statSync(testOutput);
      1063 |           console.log(`Delete "${testOutput}"`);
    > 1064 |         }).toThrow();
           |            ^
      1065 |
      1066 |         await checkSourceMap(sourceMap, ["--output", testOutput, "--source-map"]);
      1067 |         expect(fs.statSync(testOutput)).toBeInstanceOf(fs.Stats);

      at Object.<anonymous> (test/cli/run.spec.ts:1064:12)

  ● Command Line Interface › handles source map › with default name with --output › worked together with --test/--test-file

    expect(received).toThrow()

    Received function did not throw

      1077 |           // Make sure the file isn't there before we start
      1078 |           fs.statSync(testOutput);
    > 1079 |         }).toThrow();
           |            ^
      1080 |
      1081 |         await checkSourceMap(sourceMap, ["-o", testOutput, "-t", "1", "--source-map"]);
      1082 |         expect(fs.statSync(testOutput)).toBeInstanceOf(fs.Stats);

      at Object.<anonymous> (test/cli/run.spec.ts:1079:12)

  ● Command Line Interface › handles source map › with default name with --output › hides sourceMap with hidden:

    expect(received).toThrow()

    Received function did not throw

      318 |     fs.statSync(sourceMap);
      319 |     console.log(`Delete file: "${sourceMap}"`);
    > 320 |   }).toThrow();
          |      ^
      321 |
      322 |   await exec({
      323 |     args,

      at checkSourceMap (test/cli/run.spec.ts:320:6)
      at Object.<anonymous> (test/cli/run.spec.ts:1129:15)

  ● Command Line Interface › handles source map › with specified name › generates a source map 3

    expect(received).toThrow()

    Received function did not throw

      318 |     fs.statSync(sourceMap);
      319 |     console.log(`Delete file: "${sourceMap}"`);
    > 320 |   }).toThrow();
          |      ^
      321 |
      322 |   await exec({
      323 |     args,

      at checkSourceMap (test/cli/run.spec.ts:320:6)
      at Object.<anonymous> (test/cli/run.spec.ts:1152:15)

  ● Command Line Interface › handles source map › with specified name › emits an error if used with --test/--test-file

    expect(received).toThrow()

    Received function did not throw

      1158 |           // Make sure the file isn't there before we start
      1159 |           fs.statSync(sourceMap);
    > 1160 |         }).toThrow();
           |            ^
      1161 |
      1162 |         await expect(exec({
      1163 |           args: ["-t", "1", "--source-map", sourceMap],

      at Object.<anonymous> (test/cli/run.spec.ts:1160:12)

 PASS  test/behavior/generated-parser-behavior.spec.js
--------------------------------|---------|----------|---------|---------|-------------------------------
File                            | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s             
--------------------------------|---------|----------|---------|---------|-------------------------------
All files                       |    97.7 |    92.68 |   98.43 |   99.01 |                               
 bin                            |   95.69 |     92.1 |     100 |   95.58 |                               
  opts.js                       |   98.16 |       96 |     100 |   98.16 | 322,327                       
  peggy-cli.js                  |   96.83 |    93.97 |     100 |   96.74 | 317-318,351-360,446           
  peggy.js                      |   47.05 |    33.33 |     100 |   47.05 | 11-25                         
  utils.js                      |   97.36 |    85.71 |     100 |   97.14 | 132                           
  watcher.js                    |     100 |    95.45 |     100 |     100 | 49                            
 lib                            |   97.45 |    90.34 |   97.36 |   99.66 |                               
  grammar-error.js              |     100 |      100 |     100 |     100 |                               
  grammar-location.js           |     100 |      100 |     100 |     100 |                               
  parser.js                     |   97.35 |    89.91 |   97.04 |   99.64 | 92-93,624-628,640-644,734-735 
  peg.js                        |     100 |      100 |     100 |     100 |                               
  version.js                    |     100 |      100 |     100 |     100 |                               
 lib/compiler                   |      99 |    96.37 |    98.7 |   98.95 |                               
  asts.js                       |      95 |    88.88 |   92.85 |   94.73 | 97,115                        
  index.js                      |     100 |      100 |     100 |     100 |                               
  intern.js                     |     100 |      100 |     100 |     100 |                               
  opcodes.js                    |     100 |      100 |     100 |     100 |                               
  session.js                    |     100 |      100 |     100 |     100 |                               
  stack.js                      |   98.78 |    95.65 |     100 |   98.73 | 118                           
  utils.js                      |     100 |      100 |     100 |     100 |                               
  visitor.js                    |     100 |      100 |     100 |     100 |                               
 lib/compiler/passes            |   98.97 |    97.09 |    98.9 |   98.95 |                               
  add-imported-rules.js         |     100 |      100 |     100 |     100 |                               
  fix-library-numbers.js        |     100 |      100 |     100 |     100 |                               
  generate-bytecode.js          |   98.03 |    93.83 |   97.91 |   97.98 | 492-493,734                   
  generate-js.js                |   98.76 |     97.2 |   98.46 |   98.74 | 1481-1489,1493,1519           
  inference-match-result.js     |   98.48 |    98.03 |     100 |   98.41 | 160                           
  merge-character-classes.js    |     100 |      100 |     100 |     100 |                               
  remove-proxy-rules.js         |     100 |      100 |     100 |     100 |                               
  remove-unused-rules.js        |     100 |      100 |     100 |     100 |                               
  report-duplicate-imports.js   |     100 |      100 |     100 |     100 |                               
  report-duplicate-labels.js    |     100 |      100 |     100 |     100 |                               
  report-duplicate-rules.js     |     100 |      100 |     100 |     100 |                               
  report-incorrect-plucking.js  |     100 |      100 |     100 |     100 |                               
  report-infinite-recursion.js  |     100 |      100 |     100 |     100 |                               
  report-infinite-repetition.js |     100 |      100 |     100 |     100 |                               
  report-undefined-rules.js     |     100 |      100 |     100 |     100 |                               
--------------------------------|---------|----------|---------|---------|-------------------------------

Summary of all failing tests
 FAIL  test/cli/run.spec.ts
  ● Command Line Interface › outputs to a file

    expect(received).toThrow()

    Received function did not throw

      814 |       // Make sure the file isn't there before we start
      815 |       fs.statSync(test_output);
    > 816 |     }).toThrow();
          |        ^
      817 |
      818 |     await exec({
      819 |       args: ["-o", test_output],

      at Object.<anonymous> (test/cli/run.spec.ts:816:8)

  ● Command Line Interface › handles source map › with default name without --output › generates a source map 1

    expect(received).toThrow()

    Received function did not throw

      318 |     fs.statSync(sourceMap);
      319 |     console.log(`Delete file: "${sourceMap}"`);
    > 320 |   }).toThrow();
          |      ^
      321 |
      322 |   await exec({
      323 |     args,

      at checkSourceMap (test/cli/run.spec.ts:320:6)
      at Object.<anonymous> (test/cli/run.spec.ts:1024:15)

  ● Command Line Interface › handles source map › with default name without --output › emits an error if used with --test/--test-file

    expect(received).toThrow()

    Received function did not throw

      1030 |           // Make sure the file isn't there before we start
      1031 |           fs.statSync(sourceMap);
    > 1032 |         }).toThrow();
           |            ^
      1033 |
      1034 |         await expect(exec({
      1035 |           args: ["-t", "1", "--source-map"],

      at Object.<anonymous> (test/cli/run.spec.ts:1032:12)

  ● Command Line Interface › handles source map › with default name with --output › generates a source map 2

    expect(received).toThrow()

    Received function did not throw

      1062 |           fs.statSync(testOutput);
      1063 |           console.log(`Delete "${testOutput}"`);
    > 1064 |         }).toThrow();
           |            ^
      1065 |
      1066 |         await checkSourceMap(sourceMap, ["--output", testOutput, "--source-map"]);
      1067 |         expect(fs.statSync(testOutput)).toBeInstanceOf(fs.Stats);

      at Object.<anonymous> (test/cli/run.spec.ts:1064:12)

  ● Command Line Interface › handles source map › with default name with --output › worked together with --test/--test-file

    expect(received).toThrow()

    Received function did not throw

      1077 |           // Make sure the file isn't there before we start
      1078 |           fs.statSync(testOutput);
    > 1079 |         }).toThrow();
           |            ^
      1080 |
      1081 |         await checkSourceMap(sourceMap, ["-o", testOutput, "-t", "1", "--source-map"]);
      1082 |         expect(fs.statSync(testOutput)).toBeInstanceOf(fs.Stats);

      at Object.<anonymous> (test/cli/run.spec.ts:1079:12)

  ● Command Line Interface › handles source map › with default name with --output › hides sourceMap with hidden:

    expect(received).toThrow()

    Received function did not throw

      318 |     fs.statSync(sourceMap);
      319 |     console.log(`Delete file: "${sourceMap}"`);
    > 320 |   }).toThrow();
          |      ^
      321 |
      322 |   await exec({
      323 |     args,

      at checkSourceMap (test/cli/run.spec.ts:320:6)
      at Object.<anonymous> (test/cli/run.spec.ts:1129:15)

  ● Command Line Interface › handles source map › with specified name › generates a source map 3

    expect(received).toThrow()

    Received function did not throw

      318 |     fs.statSync(sourceMap);
      319 |     console.log(`Delete file: "${sourceMap}"`);
    > 320 |   }).toThrow();
          |      ^
      321 |
      322 |   await exec({
      323 |     args,

      at checkSourceMap (test/cli/run.spec.ts:320:6)
      at Object.<anonymous> (test/cli/run.spec.ts:1152:15)

  ● Command Line Interface › handles source map › with specified name › emits an error if used with --test/--test-file

    expect(received).toThrow()

    Received function did not throw

      1158 |           // Make sure the file isn't there before we start
      1159 |           fs.statSync(sourceMap);
    > 1160 |         }).toThrow();
           |            ^
      1161 |
      1162 |         await expect(exec({
      1163 |           args: ["-t", "1", "--source-map", sourceMap],

      at Object.<anonymous> (test/cli/run.spec.ts:1160:12)


Test Suites: 1 failed, 28 passed, 29 total
Tests:       8 failed, 1324 passed, 1332 total
Snapshots:   0 total
Time:        5.397 s
Ran all test suites.

hildjj · 2025-04-21T16:31:43Z

Oh. That's because you had a test fail, and you need to clean up the files that got left over. Delete the files that the error says, and it it should work cleanly.

hildjj

Two minor nits and this is ready to go.

docs/js/examples.js

test/unit/parser.spec.js

Fixed.

hildjj · 2025-04-22T23:16:54Z

Thanks!

frostburn force-pushed the soft-mode branch from 40840fa to 75b83f4 Compare March 5, 2024 08:57

frostburn force-pushed the soft-mode branch 3 times, most recently from 2cb51c0 to 5efe716 Compare April 18, 2025 18:15

hildjj requested changes Apr 18, 2025

View reviewed changes

frostburn force-pushed the soft-mode branch from 5efe716 to 1206e5b Compare April 21, 2025 15:03

frostburn mentioned this pull request Apr 22, 2025

Add missing console log for recovering failed test sessions #604

Merged

hildjj previously requested changes Apr 22, 2025

View reviewed changes

docs/js/examples.js Outdated Show resolved Hide resolved

test/unit/parser.spec.js Show resolved Hide resolved

Make it possible to throw syntax errors in library mode

db49d4a

frostburn force-pushed the soft-mode branch from 1206e5b to db49d4a Compare April 22, 2025 18:15

hildjj merged commit 5a2f9ac into peggyjs:main Apr 22, 2025
10 checks passed

frostburn deleted the soft-mode branch April 23, 2025 01:02

hildjj mentioned this pull request Apr 24, 2025

Implement soft-mode with access to partial results #501

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Soft mode #502

Soft mode #502

Soft mode #502

Soft mode #502

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment