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

Case sensitivity fix #54 #66

Merged
merged 23 commits into from
Sep 27, 2021
Merged

Conversation

g-rauhoeft
Copy link
Contributor

This pull request fixes case sensitivity for the Express Parser in accordance with ISO-10303-11, as discussed under #54

This was achieved by restricting identifiers to anything but reserved keywords, which is defined in the standard. It also changes the order of operations for certain alt parsers, because some are implemented in a way that they take precedence over subsequent parsers if case sensitivity is ignored.

…SS keywords in lowercase and a parser that checks whether the following token is a reserved keyword.
…g upper- and lowercase characters in IDs makes attribute_id recognize things that are actually redeclared_attributes.
…termination condition. This allows parsing of UNIQUE without accidentally reading keywords.
…d named_types, because by allowing uppercase and lowercase, one swallows the other.
…. This is useful when expecting a keyword in unit tests.
…o errors when allowing upper and lowercase characters.
…rors when allowing upper and lowercase characters.
@g-rauhoeft
Copy link
Contributor Author

Could you please let me know if this pull request is acceptable? A lot of fixes I'm working on depend on the changes I've made here and it would be a shame to put more effort in to something that won't be accepted.

@ytanimura
Copy link
Contributor

We apologize for not responding to you earlier.
The main developer @termoshtt of ruststep is recently very busy, and cannot check the commits precisely now.
We will announce the deadline for determination as soon as possible, so please wait a little longer.

@termoshtt
Copy link
Contributor

Could you please let me know if this pull request is acceptable?

To merge this PR requires me re-reading and understanding the ISO-10303-11 with discussed in #54, and it takes more time for review.

A lot of fixes I'm working on depend on the changes I've made here and it would be a shame to put more effort in to something that won't be accepted.

This pull requrest has some independent parts e.g. eof or not parsers. If you need these parts in other branches, It can be merged independently when you split them into individual pull requests.

@g-rauhoeft
Copy link
Contributor Author

Thank you for your reply. As for the part on case sensitivity, the definition is under 7.1.2 Letters. Throughout the document I have seen several examples of EXPRESS keywords being written in either upper or lower case. 6.1 The syntax of the specification refers to keywords being in uppercase, but that is only for the syntax of the specification in WSN, not for EXPRESS as such.

I'll try and split my pull request in to smaller pieces, but the main aim of this PR was to get case-insensitivity working, which is quite a large operation on the code base. My subsequent edits with which I managed to parse serveral APs rely on case-insensitivity unfortunately.

@CLAassistant
Copy link

CLAassistant commented Sep 21, 2021

CLA assistant check
All committers have signed the CLA.

@termoshtt termoshtt mentioned this pull request Sep 24, 2021
termoshtt added a commit that referenced this pull request Sep 27, 2021
@termoshtt termoshtt merged commit b5cab0e into ricosjp:master Sep 27, 2021
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.

4 participants