Skip to content
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

WIP: addding EBNF #54

Closed
wants to merge 143 commits into from
Closed

WIP: addding EBNF #54

wants to merge 143 commits into from

Conversation

SKalt
Copy link
Contributor

@SKalt SKalt commented Nov 14, 2019

Work on #17. This is a place to show how my work's progressing.

Snewt and others added 30 commits October 22, 2017 20:25
Prior to this commit there were some documention bugs,
spelling errors and extra words left in from previous revisions.
There may of course be more, but after this commit the ones I've
caught won't pester me anymore.
Add some more nice badges to the README
Previously the BNF types implemented the ToString trait. However the
rust convention is to implement fmt::Display instead because it covers
wider use cases and automatically implements the ToString trait as well.
Now the types support format as well as to_string.
…of-toString

 Implement format display instead of ToString
Prior to this commit there was no code testing coverage report
generated which I consider valuable. Coveralls and a package called
`cargo-travis` made it possible for me to integrate that information
into each build. This commit represents that integration and badges
to go along with the report.
Prior to this commit there was no test to provide assurances that changes
to the codebase would not effect the expected output implemented as the
Display trait. This commit adds a test to help provide that assurance.
shnewto and others added 22 commits January 15, 2019 15:47
Fix for redzone comparison error
Doc formatting and typo fixes. Remove const name case warning
Derive Eq and Hash for some work on a dependent crate
Added len() function for Production to return number of rhs Expressions
There should be no functional changes from this migration, however some of the parsers needed to be updated to handle changes in nom.
…added to to indicate it's unused as well as fix a warning
`an_array.into_iter()` currently just works because of the autoref
feature, which then calls `<[T] as IntoIterator>::into_iter`. But
in the future, arrays will implement `IntoIterator`, too. In order
to avoid problems in the future, the call is replaced by `iter()`
which is shorter and more explicit.
Use `slice::iter` instead of `into_iter` to avoid future breakage
* saving state mid transations, unsure what to do about the old eof biz

* saving state

* wrap up transitioning from nom 3.x logic to nom 5.1, address some linting warnings along the way. most notably, removed the weird stuff I'd been doing by implementing each object's own from_str

* another change the linter asked for
…e warnings (shnewto#51)

* removed custom from_str impl in favor of just implementing the FromStr trait

* update docs to prefer  to

* Set timeout to 30 min (instead of default 360 min)

Builds were taking between and 2.5 hours and 3.5 hours so trying to address that.
@prbs23
Copy link

prbs23 commented Nov 19, 2019

This is great! I'm glad to see someone else looking at this area.
I had started working on this problem as well, but only got as far as implementing the parsers. Integrating some of the ebnf specific features (repetitions, character sets, etc.) into the existing data structures got a little tricky and I ran out of free time to work on it. If you are interested, you can check out the work I had done so far in this fork: https://github.com/prbs23/bnf/tree/ebnf_parser

@SKalt
Copy link
Contributor Author

SKalt commented Apr 15, 2020

Hey @prbs23, I'm interested in picking up your work again. Do you remember how you were thinking of lotting your newer EBNF architecture into the existing data structures?

@prbs23
Copy link

prbs23 commented Jun 24, 2020

Hey @SKalt, it's been a while since I thought about this, let's see how much I remember.
In the ebnf_parser branch of my fork (linked above) inside ebnf_w3c_parsers.rs I have defined the local copies of all the data structures and internal fields that I thought were needed to represent an ebnf grammar, essentially the same Grammar, Production, Expression, and Term structures that are currently used. I think the structures just need to be expanded/modified some to handle additional types of expressions.

The biggest change is for ebnf there's four different types of expressions, terms, difference, sequence, and choice, instead of just being a list of terms. I was thinking I would convert the existing Expression struct into an enum to capture these four different types of expressions.
The other difference is is that ebnf has a new type of "Term", which is a character class. This isn't so big of a change from the current Term structure, since it's already an enum that can represent a Terminal or NonTerminal term value.
I think the Grammar and Production structures can remain essentially the same. The only difference I think that was needed was that the rhs value of the Production can now just be a single Expression enum instead of a list of Expressions.

I never got far enough into actually making these changes to know what kinds unforeseen issues there would be with this proposal. I'm sure there's something I'm overlooking since I'm not that familiar with this code base, but I think it's a reasonable starting point.

@shnewto
Copy link
Owner

shnewto commented Jul 29, 2020

👋 Hey @SKalt (and @prbs23 since you're here too) I need to revise some of this repo's early history and wanted to give you both a heads up so you aren't surprised by the wild complaints it'll cause the next time you try to sync up. I'd say minimally, make a back up before trying to pull anything in 🙃 And let me know if you have any issues, I'm happy to try and help out.

@shnewto
Copy link
Owner

shnewto commented Jul 29, 2020

@SKalt looks me kicking off the history revisions caused this to close automatically 🤔 Feel free to open it up again or make another any time.

@shnewto shnewto mentioned this pull request Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants