-
-
Notifications
You must be signed in to change notification settings - Fork 414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Load files once #998
base: master
Are you sure you want to change the base?
Load files once #998
Conversation
…rammar during a single call to `load_grammar`
@MegaIng say i have 2 grammars A and B that both include a grammar C. now if i include grammars A and B in a grammar D, i get conflict errors as rules defined in grammar C are declared twice. is that wanted? |
@ornariece Without a major rewrite of The logic is that you can manually specify |
42ab48a
to
47124f9
Compare
@erezsh This PR is now done. I will put |
@MegaIng you added |
7ed9b36
to
1d8237e
Compare
HOW? I completely reverted those changes (they aren't even in my local files). I am not sure where git got those changes from... |
@MegaIng (currently tweaking my grammars to use |
@ornariece That is part of the reason why I removed this implementation of |
@MegaIng I'm probably missing something, but why not just make it use the same implementation of |
@erezsh That leads exactly to the problem @ornariece describes, since imports (and then includes) are added before anything in the grammar is done, meaning that an included grammar could not extend symbols in the main grammar. Also, that would still create a full |
I think that's the desirable behavior. Perhaps these symbols should be refactored into a |
I disagree. Including a grammar should behave no different then just pasting the entire content at that place. |
For that behavior, it has to be processed before all other directives. |
@erezsh That is the plan, but that is interdependent of this PR. I will create one for those changes. |
Sorry I'm only getting to this now. Any chance you can fix the PR for the recent changes? It might even need a few more changes for allowing abnf and such? (maybe not. I didn't look that deeply yet) |
The changes implemented during the discussion of #992, and a basic implementation of
%include
(typo in commit message :-/). Should I also implement more complex load once stuff in this PR?