-
Notifications
You must be signed in to change notification settings - Fork 780
Add support for OCaml. #4758
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 support for OCaml. #4758
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
Thank you for signing the OCA. |
opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java
Outdated
Show resolved
Hide resolved
opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/ocaml/Consts.java
Outdated
Show resolved
Hide resolved
opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/ocaml/OCamlAnalyzer.java
Outdated
Show resolved
Hide resolved
opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/ocaml/OCamlAnalyzerFactory.java
Outdated
Show resolved
Hide resolved
opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/ocaml/OCamlLexer.java
Outdated
Show resolved
Hide resolved
opengrok-indexer/src/main/jflex/analysis/ocaml/OCamlSymbolTokenizer.lex
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in general this looks good, the copyright commens have to be addressed.
Also, it would be nice to supply a sample OCaml file + expected xref and add a test checking it, similarly to some other analyzers. |
Will add within couple days. Could you please give a good example? |
See the recently changed Fortran analyzer (PR #4642) for example. |
@vladak thanks for the hints. Added tests, please take a look. |
There is a pre-existing issue tracking the OCaml support (#4407), please add |
Also, rebase. |
opengrok-indexer/src/main/jflex/analysis/ocaml/OCamlSymbolTokenizer.lex
Outdated
Show resolved
Hide resolved
opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/ocaml/OCamlXrefTest.java
Outdated
Show resolved
Hide resolved
I believe the newly introduced files do not have to have the |
opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/ocaml/OCamlXrefTest.java
Show resolved
Hide resolved
Vladimir, from my point of view, all the comments seem to have been addressed. Would you please double check if the PR can be merged? Thank you very much for your hard work on OpenGrok. |
Looks good, sorry for the delay. Thanks for the contribution ! |
Thank you so much for your review and hard work on this incredibly useful tool! |
I am signing Oracle Contributor Agreement.
This pull request adds support for OCaml.
Almost full support for OCaml syntax is provided including quoted string literals and extension nodes. The support for docstrings is not implemented. Also, primitive types are added to the list of keywords because most programmers do not use them as variable names (
result
is an exception).