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

add FreeFem++ lexer #1020

Closed
wants to merge 1 commit into from
Closed

add FreeFem++ lexer #1020

wants to merge 1 commit into from

Conversation

sgarnotel
Copy link
Contributor

@sgarnotel sgarnotel commented Oct 26, 2018

Lexer for FreeFem++ (Github repo).

This is a finite element language, C++ idiom but with a lot of difference (so, new class)

Thanks in advance for merging!

@franckl
Copy link

franckl commented Nov 27, 2018

👍 it would be very useful to have. Looking forward to it !

@pyrmont
Copy link
Contributor

pyrmont commented Jul 2, 2019

@sgarnotel I'm sorry it's taken so long to get to this PR. Thank you for submitting it!

After going over it, I wondered if, given the similarities between this lexer and the C lexer (unsurprisingly given the C++ idiom), it wouldn't be preferable to inherit from the C (or perhaps the C++) lexer class and then extend where necessary?

That would greatly cut down on the code in this lexer, hopefully making it easier to review and maintain. You can see an example of how this would work with the C++ lexer.

@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Jul 2, 2019
@pyrmont pyrmont self-assigned this Aug 26, 2019
@sgarnotel
Copy link
Contributor Author

Thanks for your reply, I opened a new PR for the inherited lexer
#1356

@sgarnotel sgarnotel closed this Nov 8, 2019
@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label Apr 4, 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.

3 participants