ohm icon indicating copy to clipboard operation
ohm copied to clipboard

Grammar syntax for imports/includes

Open Schahen opened this issue 4 years ago • 9 comments

Correct me if I'm wrong, but from what I see there's no way to include grammar definitions from one file into another one.

My questions are following:

  1. Is it indeed the case?
  2. If yes, can we discuss introduction of such feature? I'm OK with participating if needed.

Schahen avatar Jan 21 '21 21:01 Schahen

Hi @Schahen, are you referring to something like an import/include statement in the grammar itself?

E.g.,

include '../Arithmetic.ohm'

MyGrammar <: Arithmetic {
    ...
}

If so, then you are correct — it's not something we support today. But I'm definitely open to it if you have a proposal!

pdubroy avatar Jan 22 '21 08:01 pdubroy

@pdubroy Following up on this, I think this would be a really useful feature but I feel it could be done in the ohm-cli.

  1. Load the grammars from disk
  2. For each match grammar 2a. Try and process imports 2b. Attempt to load defined (if any) imports from disk using relative path. 2c. If successful Substitute the contents of the loaded file in place of the import. 2c. If error Throw FileNotFound for import error 2e. Follow existing parsing flow.

With the above in mind, is ohm-js used to parse the ohm files when pre-building grammars using the cli? If you have a better approach, please let me know.

I'd be keen to potentially look at getting this working as it's exactly what I need.

LiamRiddell avatar Aug 07 '23 14:08 LiamRiddell

As mentioned in #457, I think a complete implementation of this feature needs to address a few things.

  1. We should add the syntax to the canonical grammar for the Ohm language (ohm-grammar.ohm), rather than using a regex.
  2. Implement a way for this to work when instantiating a grammar via ohm.grammar. Otherwise, we would effectively be forking the language into one version that's supported by @ohm-js/cli, and another version that's supported in the API.
  3. Implement a solution for (3) that works in both Node and in browsers.
  4. We'd also need to make sure that when you generate TypeScript types for a grammar, it handles imports correctly.

Regarding (2), a simple approach might be something like the following:

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

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

The idea would be that when you call ohm.grammar, you can optionally pass an options object, and that object could contain a function that would be used to resolve imports. So, Ohm itself wouldn't explicitly resolve imports, but it would be easy enough to write a one-liner that handles imports in either Node or the browser.

Alternatively, we could include default implementations, but that would be a bit more complicated with regards to bundling, etc.

pdubroy avatar Aug 16 '23 10:08 pdubroy

Implemented the above, awaiting review on the linked #457

LiamRiddell avatar Aug 25 '23 20:08 LiamRiddell

Any progress on this front? No way to generateBundles of a grammar that extends another. Or is there?

marco3479 avatar Nov 08 '23 03:11 marco3479

Any progress on this front? No way to generateBundles of a grammar that extends another. Or is there?

@marco3479 You can extend grammars but they have to be defined in the same file. For me, this was too restrictive. I have since put together a proposal PR but it was awaiting review for a while and I've had to close it due to lack of time on my end. Hopefully it can be picked up again later.

What you can do is write your own pre-processing to automatically search and replace includes... I've seen others using tools for this. Otherwise, you can use my fork which is currently used by my project Solve.

LiamRiddell avatar Nov 08 '23 18:11 LiamRiddell

Noted. Thanks @LiamRiddell.

marco3479 avatar Nov 08 '23 22:11 marco3479

@marco3479 Can you share some details about your use case?

@LiamRiddell, I appreciate you submitting the PR and I apologize for letting it drop. I'd also like to hear a bit more about the specifics of your use case.

I would like to figure out how to best support this, but I had second thoughts about the solution I proposed. It complicates the API and language more than I'd like.

pdubroy avatar Nov 10 '23 19:11 pdubroy

@pdubroy, I have a two grammars; B extends A. I was trying to keep the grammar definitions in their own .ohm files. When trying to generate the TS bundle for B, reasonably, it wouldn't recognize A (as was defined elsewhere). Adding the import syntax should fix this. Honestly, the only minor bother regarding this is that I can't keep the grammars isolated, rather I have to define in a single .ohm file, and if I make updates to any of the grammars, I have to generate TS bundles for both simultaneously. Unless I'm missing something, it's not a deal-breaker.

marco3479 avatar Nov 13 '23 22:11 marco3479