LiamRiddell commented 1 year ago

Fixes #286

I've implemented a fix that prevents VS Code complaining about LF vs CLRF line endings. This uses the solution provided in this article and enforces LF line endings.

Implementation (2.0)

[X] Revert the original commit for ohm-cli
[X] Add a new grammar rule for include '...' in the ohm-js grammar
[x] Introduce unit tests for the new include statement in the grammar
[x] Implement visitor for Document, Includes, and Include rules
[x] Implement optional options object for the ohm.grammar function in ohm-js
[x] Add new option called fetchImport that is provided via optional options object (Add circuit breaker for preventing infinite loop includes)
[ ] Add default implementation for Node and Browser, utilising .node.js and .browser.js file type support (optional)

Thoughts

[ ] How does this work when bundling with CLI for TypeScript? I feel we should have two modes, resolve and passthrough mode.

pdubroy commented 1 year ago

Hi Liam,

Thanks for the putting in the work to prototype this! Adding support for imports is something we've thought about over the years, but have hesitated to implement. Mainly, my concern is — what happens if someone is trying to load the grammar via ohm.grammar(...) rather than using the CLI? And then, the question is how to handle imports when Ohm is running in Node, vs. the browser?

This is a feature that I'd be happy to add, but if we're going to do it, I'd like to make sure we think it through and implement it fully. If it's something you're up for, we could collaborate on it.

I'll leave some notes on #286 about some of the things I'd like to address in a complete implementation.

pdubroy commented 1 year ago

Whoops, didn't mean to close this PR. I think we keep it open and iterate on it.

LiamRiddell commented 1 year ago

Hi Liam,

Thanks for the putting in the work to prototype this! Adding support for imports is something we've thought about over the years, but have hesitated to implement. Mainly, my concern is — what happens if someone is trying to load the grammar via ohm.grammar(...) rather than using the CLI? And then, the question is how to handle imports when Ohm is running in Node, vs. the browser?

This is a feature that I'd be happy to add, but if we're going to do it, I'd like to make sure we think it through and implement it fully. If it's something you're up for, we could collaborate on it.

I'll leave some notes on #286 about some of the things I'd like to address in a complete implementation.

Hi Patrick,

I'd love to collaborate, this as you mentioned was just a fix to get it working for my use case but I was keen to bring it in language as opposed to a CLI-only option.

ohm.grammar(`
  include './foo.ohm'

  G {
    ...
  }
`, {
  fetchGrammar(relativeUrl) {
    return fs.readFileSync(relativeUrl, import.meta.url, 'utf-8');
  }
})

This is a good starting point. Which allows the user to provide the method of resolution of the imported grammar. However, I'd expect the most common implementation to be:

Node - Read the file from disk based on the relative.
Browser - Fetched from url based on the relative path.

Which I believe we could provide default implementations for e.g. defaultFetchImport.node.js and defaultFetchImport.browser.js.

Next steps

I'll add a new grammar rule for include '...' in the ohm-js grammar including unit tests
Implement a new options object for the ohm.grammar function.
Add a new options for fetchImport

We then can evaluate the above and see how this would play with TypeScript and then make a new iteration with new learnings.

LiamRiddell commented 1 year ago

@pdubroy I'm trying to get the grammar working with the existing unit tests. However, I'm having issues with the test setup. A few of the test files are referencing files that do not exist. What am I missing here? Ignore the valid failing tests.

Test Steps

pnpm prebootstrap
pnpm run bootstrap
pnpm test


/workspaces/ohm/packages/ohm-js (main) $ pnpm test

> ohm-js@17.1.0 test /workspaces/ohm/packages/ohm-js
> ava && node test/_test-doc.js

  ✖ extras › extractExamples › empty Error thrown in test
  ✖ extras › extractExamples › grammar with no examples Error thrown in test
  ✖ extras › extractExamples › simple positive examples Error thrown in test
  ✖ extras › extractExamples › examples for default start rule Error thrown in test
  ✖ extras › extractExamples › whitespace in trailing example comments Error thrown in test
  ✖ extras › extractExamples › example comments - negative examples Error thrown in test
  ✖ extras › extractExamples › example comments - corner cases Error thrown in test
  ✖ extras › extractExamples › extracted examples Error thrown in test
  ✖ examples › math example Error thrown in test
  ✖ examples › viz example Error thrown in test
  ✖ examples › csv example 
  ✔ built-in-rules › case-insensitive matching (298ms)
  ✔ built-in-rules › applySyntactic - basics (186ms)
  ✔ built-in-rules › applySyntactic - space skipping (111ms)
  ✔ built-in-rules › applySyntactic with lookahead
  ✔ errors › match failure (384ms)
  ✔ errors › undeclared rules (357ms)
  ✔ errors › many expressions with nullable operands (348ms)
  ✔ errors › errors from ohm.grammar() (225ms)
  ✔ errors › unrecognized escape sequences (219ms)
  ✔ errors › failures are memoized (194ms)
  ✔ errors › multiple MatchResults from the same Matcher (185ms)
  ✔ errors › non-fluffy failures subsume fluffy failures, etc. (141ms)
  ✔ errors › memo recs that do not contain the necessary info are deleted properly
  ✔ errors › trailing space should not influence the result
  ✔ errors › method name displayed on abstract function failure
  ✔ errors › errors for Not-of-<PExpr>
  ✔ errors › complex match failure
  ✔ errors › wrongNumberOfArguments includes the interval
  ✔ findIndentation › findIndentationPoints
  ✔ incremental › basic incremental parsing (275ms)
  ✔ incremental › trickier incremental parsing (256ms)
  ✔ incremental › examinedLength - no LR (228ms)
  ✔ incremental › examinedLength - no LR, but non-monotonic (196ms)
  ✔ incremental › examinedLength - simple LR (191ms)
  ✔ incremental › examinedLength - complicated LR (172ms)
  ✔ incremental › rightmostFailureOffset - no LR (155ms)
  ✔ incremental › rightmostFailureOffset - simple LR (151ms)
  ✔ incremental › rightmostFailureOffset - complicated LR (127ms)
  ✔ incremental › matchLength (124ms)
  ✔ incremental › matchLength - complicated LR (108ms)
  ✔ incremental › binding offsets - lexical rules
  ✔ incremental › binding offsets - syntactic rules
  ✔ incremental › incremental parsing + attributes = incremental computation
  ✔ grammar › action dictionary templates (302ms)
  ✔ grammar › default start rule (221ms)
  ✔ grammar › grammar equality (108ms)
  ✔ input-stream › next() at end
  ✔ indentation-sensitive › all dedents must be consumed (131ms)
  ✔ indentation-sensitive › can't skip over an indent (111ms)
  ✔ indentation-sensitive › any consumes indent and dedent
  ✔ indentation-sensitive › incremental parsing fails
  ✔ indentation-sensitive › memoization
  ✔ indentation-sensitive › basic tracing
  ✔ interval › collapsedLeft
  ✔ interval › collapsedRight
  ✔ interval › coverage - one interval
  ✔ interval › two adjacent intervals
  ✔ interval › coverage - two non-adjacent intervals
  ✔ interval › coverage - nested intervals
  ✔ interval › coverage - more intervals
  ✔ interval › brotha from anotha motha
  ✔ interval › coverageWith
  ✔ interval › getLineAndColumn
  ✔ main › namespaces (185ms)
  ✔ main › plain JS objects as namespaces (137ms)
  ✔ main › instantiating grammars from different types of objects
  ✔ parameterized-rules › require same number of params when overriding and extending (323ms)
  ✔ parameterized-rules › require same number of args when applying (244ms)
  ✔ parameterized-rules › require arguments to have arity 1 (194ms)
  ✔ parameterized-rules › require the rules referenced in arguments to be declared (184ms)
  ✔ parameterized-rules › simple examples (161ms)
  ✔ parameterized-rules › start matching from parameterized rule (123ms)
  ✔ parameterized-rules › inline rule declarations
  ✔ parameterized-rules › left recursion
  ✔ parameterized-rules › complex parameters
  ✔ parameterized-rules › duplicate parameter names
  ✔ ohm-syntax › char (906ms)
  ✔ ohm-syntax › string (882ms)
  ✔ ohm-syntax › unicode code point escapes (849ms)
  ✔ ohm-syntax › unicode recognition (778ms)
  ✔ ohm-syntax › unicode semantic actions (769ms)
  ✔ ohm-syntax › ranges (768ms)
  ✔ ohm-syntax › ranges w/ code points > 0xFFFF (738ms)
  ✔ ohm-syntax › ranges w/ code points > 0xFFFF, special cases (724ms)
  ✔ ohm-syntax › any consumes an entire code point (717ms)
  ✔ ohm-syntax › alt recognition (707ms)
  ✔ ohm-syntax › alt semantic actions (707ms)
  ✔ ohm-syntax › rule bodies in defs can start with a |, and it's a no-op recognition (706ms)
  ✔ ohm-syntax › rule bodies in defs can start with a |, and it's a no-op semantic actions (706ms)
  ✔ ohm-syntax › rule bodies in overrides can start with a |, and it's a no-op recognition (706ms)
  ✔ ohm-syntax › rule bodies in overrides can start with a |, and it's a no-op semantic actions (706ms)
  ✔ ohm-syntax › rule bodies in extends can start with a |, and it's a no-op recognition (706ms)
  ✔ ohm-syntax › rule bodies in extends can start with a |, and it's a no-op semantic actions (705ms)
  ✔ ohm-syntax › seq recognition (705ms)
  ✔ ohm-syntax › seq semantic actions (705ms)
  ✔ ohm-syntax › alts and seqs together recognition (695ms)
  ✔ ohm-syntax › alts and seqs together semantic actions (695ms)
  ✔ ohm-syntax › kleene-* and kleene-+ recognition (693ms)
  ✔ ohm-syntax › kleene-* and kleene-+ semantic actions (693ms)
  ✔ ohm-syntax › kleene-* and kleene-+ semantic actions are evaluated lazily (675ms)
  ✔ ohm-syntax › opt recognition (674ms)
  ✔ ohm-syntax › opt semantic actions (673ms)
  ✔ ohm-syntax › not recognition (673ms)
  ✔ ohm-syntax › not semantic actions (673ms)
  ✔ ohm-syntax › lookahead recognition (672ms)
  ✔ ohm-syntax › lookahead semantic actions (672ms)
  ✔ ohm-syntax › simple left recursion recognition (671ms)
  ✔ ohm-syntax › simple left recursion semantic actions (670ms)
  ✔ ohm-syntax › simple left recursion, with non-involved rules recognition (661ms)
  ✔ ohm-syntax › simple left recursion, with non-involved rules semantic actions (661ms)
  ✔ ohm-syntax › indirect left recursion recognition (660ms)
  ✔ ohm-syntax › indirect left recursion semantic actions (656ms)
  ✔ ohm-syntax › nested left recursion recognition (655ms)
  ✔ ohm-syntax › nested left recursion semantic actions (654ms)
  ✔ ohm-syntax › nested left recursion semantic actions are evaluated lazily (652ms)
  ✔ ohm-syntax › nested and indirect left recursion recognition (643ms)
  ✔ ohm-syntax › nested and indirect left recursion semantic actions (642ms)
  ✔ ohm-syntax › tricky left recursion (different heads at same position) recognition (641ms)
  ✔ ohm-syntax › tricky left recursion (different heads at same position) semantic actions (641ms)
  ✔ ohm-syntax › no namespace (640ms)
  ✔ ohm-syntax › empty namespace (638ms)
  ✔ ohm-syntax › duplicate definition (637ms)
  ✔ ohm-syntax › override it checks that rule exists in super-grammar (620ms)
  ✔ ohm-syntax › override shouldn't matter if arities aren't the same (618ms)
  ✔ ohm-syntax › override should be ok to add new cases (612ms)
  ✔ ohm-syntax › override recognition (610ms)
  ✔ ohm-syntax › override semantic actions (609ms)
  ✔ ohm-syntax › extend recognition (609ms)
  ✔ ohm-syntax › extend semantic actions (608ms)
  ✔ ohm-syntax › extend should check that rule exists in super-grammar (608ms)
  ✔ ohm-syntax › extend should make sure rule arities are compatible (606ms)
  ✔ ohm-syntax › extend should be ok to add new cases (597ms)
  ✔ ohm-syntax › override with "..." (595ms)
  ✔ ohm-syntax › bindings inconsistent arity in alts is an error (544ms)
  ✔ ohm-syntax › bindings by default, bindings are evaluated lazily (541ms)
  ✔ ohm-syntax › inline rule declarations (532ms)
  ✔ ohm-syntax › lexical vs. syntactic rules can't call syntactic rule from lexical rule, not not the other way around (473ms)
  ✔ ohm-syntax › lexical vs. syntactic rules lexical rules don't skip spaces implicitly (465ms)
  ✔ ohm-syntax › lexical vs. syntactic rules syntactic rules skip spaces implicitly (443ms)
  ✔ ohm-syntax › lexical vs. syntactic rules mixing lexical and syntactic rules works as expected (442ms)
  ✔ ohm-syntax › lexical vs. syntactic rules lexification operator works as expected (439ms)
  ✔ ohm-syntax › space skipping semantics (423ms)
  ✔ ohm-syntax › single-line comment after case name (#282) (419ms)
  ✔ ohm-syntax › bootstrap it can recognize arithmetic grammar (405ms)
  ✔ ohm-syntax › bootstrap it can recognize itself (398ms)
  ✔ ohm-syntax › bootstrap it can produce a grammar that works (320ms)
  ✔ ohm-syntax › bootstrap full bootstrap! (180ms)
  ✔ pexprs › rule definition source (175ms)
  ✔ pexprs › rule application source (144ms)
  ✔ pexprs › toDisplayString (103ms)
  ✔ pexprs › toString
  ✔ pexprs › toArgumentNameList
  ✔ recipes › simple grammar recipes (224ms)
  ✔ recipes › grammar recipes with supergrammars (205ms)
  ✔ recipes › grammar recipes involving parameterized rules (144ms)
  ✔ recipes › grammar recipes with source (116ms)
  ✔ recipes › recipes with U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR
  ✔ recipes › semantics recipes
  ✔ recipes › semantics recipes (special cases)
  ✔ recipes › semantics recipes with extensions
  ✔ recipes › semantics recipes w/ method shorthand
  ✔ recipes › recipes with astral plane code units
  ✔ semantics › operations (451ms)
  ✔ semantics › operations with arguments (350ms)
  ✔ semantics › attributes (259ms)
  ✔ semantics › attributes - same-named attributes don't collide (230ms)
  ✔ semantics › semantics (218ms)
  ✔ semantics › _iter nodes (129ms)
  ✔ semantics › _terminal nodes (122ms)
  ✔ semantics › semantic action arity checks (113ms)
  ✔ semantics › extending semantics
  ✔ semantics › mixing nodes from one grammar with semantics from another
  ✔ semantics › asIteration
  ✔ semantics › sourceString
  ✔ semantics › sourceString - issue #188
  ✔ semantics › sourceString - issue #204
  ✔ semantics › action call stacks
  ✔ semantics › error message for incorrect _iter or _nonterminal arity
  ✔ semantics › toString on an inner NodeWrapper - issue #416
  ✔ tracing › basic tracing (268ms)
  ✔ tracing › space skipping (252ms)
  ✔ tracing › tracing with parameterized rules (221ms)
  ✔ tracing › tracing with memoization (184ms)
  ✔ tracing › tracing with parameterized rules and memoization (163ms)
  ✔ tracing › tracing with left recursion (125ms)
  ✔ tracing › tracing with left recursion and leading space (108ms)
  ✔ tracing › toString
  ✔ tracing › toString with left recursion
  ✔ tracing › displayString
  ✔ tracing › memoization
  ✔ tracing › bindings
  ✔ tracing › tracing with "..."
  ✔ util › getLineAndColumn().toString()
  ✔ util › getLineAndColumnMessage
  ✔ util › getLineAndColumnMessage with ranges
  ✔ examples › prettyPrint › basic
  ✔ examples › prettyPrint › grammar with supergrammar
  ✔ examples › prettyPrint › multiple rules
  ✔ extras › storedAttributes › stored attributes
  ✔ extras › visitorFamily › basic
  ✔ extras › visitorFamily › array props
  ✔ extras › visitorFamily › arity checks
  ✔ extras › visitorFamily › unknown action names
  ✔ extras › visitorFamily › unrecognized tags
  ✔ extras › visitorFamily › operations with arguments
  ✔ extras › toAST › semantic action (189ms)
  ✔ extras › toAST › default (139ms)
  ✔ extras › toAST › mapping
  ✔ extras › toAST › real examples (combinations)
  ✔ extras › toAST › usage errors
  ✔ extras › toAST › listOf and friends - #394
  ─

  extras › extractExamples › empty

  test/extras/test-extractExamples.js:5

   4: test('empty', t => {                   
   5:   t.deepEqual(extractExamples(''), []);
   6: });                                    

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › file://test/extras/test-extractExamples.js:5:15

  extras › extractExamples › grammar with no examples

  test/extras/test-extractExamples.js:9

   8: test('grammar with no examples', t => {     
   9:   t.deepEqual(extractExamples('G { }'), []);
   10: });                                         

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › file://test/extras/test-extractExamples.js:9:15

  extras › extractExamples › simple positive examples

  test/extras/test-extractExamples.js:13

   12: test('simple positive examples', t => {
   13:   let examples = extractExamples(`     
   14:     G {                                

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › file://test/extras/test-extractExamples.js:13:18

  extras › extractExamples › examples for default start rule

  test/extras/test-extractExamples.js:37

   36: test('examples for default start rule', t => {
   37:   let examples = extractExamples(`            
   38:     G {                                       

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › file://test/extras/test-extractExamples.js:37:18

  extras › extractExamples › whitespace in trailing example comments

  test/extras/test-extractExamples.js:61

   60:   const expected = [{grammar: 'G', rule: '', example: '', shouldMatch: true}];
   61:   t.deepEqual(extractExamples('G{  //+ ""\n  }'), expected);                  
   62:   t.deepEqual(extractExamples('G{  //+ "" \n}'), expected);                   

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › file://test/extras/test-extractExamples.js:61:15

  extras › extractExamples › example comments - negative examples

  test/extras/test-extractExamples.js:67

   66: function getExamples(input) {                                                        
   67:   return extractExamples(`G { ${input}\nstart = }`).map(({example, shouldMatch}) => {
   68:     return {example, shouldMatch};                                                   

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › getExamples (file://test/extras/test-extractExamples.js:67:10)
  › file://test/extras/test-extractExamples.js:73:15

  extras › extractExamples › example comments - corner cases

  test/extras/test-extractExamples.js:67

   66: function getExamples(input) {                                                        
   67:   return extractExamples(`G { ${input}\nstart = }`).map(({example, shouldMatch}) => {
   68:     return {example, shouldMatch};                                                   

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › getExamples (file://test/extras/test-extractExamples.js:67:10)
  › file://test/extras/test-extractExamples.js:89:7

  extras › extractExamples › extracted examples

  test/extras/test-extractExamples.js:126

   125: test('extracted examples', t => {                                                       
   126:   for (const {grammar, rule, example, shouldMatch} of extractExamples(grammarsSource)) {
   127:     const g = grammars[grammar];                                                        

  Error thrown in test:

  missingSemanticAction (Error) {
    message: `Missing semantic action for 'Document' in operation 'examples'.␊
    Action stack (most recent call last):␊
    ␊
      examples > Document`,
  }

  › examples > Document
  › createError (file://src/errors.js:15:9)
  › Module.missingSemanticAction (file://src/errors.js:305:13)
  › Semantics.Wrapper.<anonymous> (file://src/Semantics.js:491:20)
  › Operation.execute (file://src/Semantics.js:650:39)
  › Semantics.Wrapper.doIt [as examples] (file://src/Semantics.js:368:29)
  › extractExamples (file://extras/extractExamples.js:156:33)
  › file://test/extras/test-extractExamples.js:126:55

  examples › math example

  test/test-examples.cjs:90

   89:   var srcDate = new Date(mtimes.pop());                                      
   90:   var bundleDate = fs.statSync(path.join(__dirname, '../dist/ohm.js')).mtime;
   91:                                                                              

  Error thrown in test:

  Error {
    code: 'ENOENT',
    errno: -2,
    path: '/workspaces/ohm/packages/ohm-js/dist/ohm.js',
    syscall: 'stat',
    message: 'ENOENT: no such file or directory, stat \'/workspaces/ohm/packages/ohm-js/dist/ohm.js\'',
  }

  › rebuildIfModified (test/test-examples.cjs:90:23)
  › test/test-examples.cjs:106:3

  examples › viz example

  test/test-examples.cjs:90

   89:   var srcDate = new Date(mtimes.pop());                                      
   90:   var bundleDate = fs.statSync(path.join(__dirname, '../dist/ohm.js')).mtime;
   91:                                                                              

  Error thrown in test:

  Error {
    code: 'ENOENT',
    errno: -2,
    path: '/workspaces/ohm/packages/ohm-js/dist/ohm.js',
    syscall: 'stat',
    message: 'ENOENT: no such file or directory, stat \'/workspaces/ohm/packages/ohm-js/dist/ohm.js\'',
  }

  › rebuildIfModified (test/test-examples.cjs:90:23)
  › test/test-examples.cjs:113:3

  examples › csv example

  test/test-examples.cjs:120

   119: test('csv example', function (t) {                      
   120:   t.notThrows(function () {                             
   121:     require(path.join(EXAMPLE_ROOT, 'csv', 'index.js'));

  Function threw:

  Error {
    code: 'MODULE_NOT_FOUND',
    path: '/workspaces/ohm/packages/ohm-js/package.json',
    requestPath: '../../packages/ohm-js',
    message: 'Cannot find module \'/workspaces/ohm/packages/ohm-js/dist/ohm.cjs\'. Please verify that the package.json has a valid "main" entry',
  }

  › Object.<anonymous> (/workspaces/ohm/examples/csv/index.js:8:13)

  ─

  11 tests failed
  2 known failures
 ELIFECYCLE  Test failed. See above for more details.
 ```

pdubroy commented 1 year ago

A couple of the tests depend on the generated bundles, so I think you need to run pnpm build before pnpm test.

(It's not ideal — maybe we should run it build automatically before the test, but that can result in wasted work in a lot of circumstances.)

LiamRiddell commented 1 year ago

A couple of the tests depend on the generated bundles, so I think you need to run pnpm build before pnpm test.

(It's not ideal — maybe we should run it build automatically before the test, but that can result in wasted work in a lot of circumstances.)

That makes sense, I just ran a pnpm build followed by pnpm build and it's working as expected with all but two tests passing.

tracing › memoization
tracing › bindings

Do these usually fail?

pdubroy commented 1 year ago

Do these usually fail?

Yes, those are marked as known failures. The output is slightly confusing but ava allows you to mark tests as "known failures" so that pnpm test succeeds (= return code 0) but still reports those as known failures.

LiamRiddell commented 1 year ago

Do these usually fail?

Yes, those are marked as known failures. The output is slightly confusing but ava allows you to mark tests as "known failures" so that pnpm test succeeds (= return code 0) but still reports those as known failures.

Interesting, thanks for the insight. That makes sense to why I'm getting the return code 0. I'm looking at implementing unit tests now for the grammar modifications.

LiamRiddell commented 1 year ago

@pdubroy It would be good to do a partial review now. All tests are passing ✔️

Side notes:

I think it would make sense to look at bringing the namespace options into the options object I've created. However, let me know your thoughts?
I did some investigation, in the current setup I don't think it's easily possible to provide a default fetchGrammar implementation for node and the browser.

ohmjs / ohm

Add Grammar Include Support #457

Implementation (2.0)

Thoughts

Next steps

Test Steps