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 support for $ in fortran identifiers and clickable .i include files. #4642

Merged
merged 10 commits into from
Oct 4, 2024

Conversation

navinp0304
Copy link
Contributor

Implementation according to discussion based on vladak #4610
I've verified that the include 'file' are clickable.
I've signed the oca.

Implementation according to discussion based on vladak
#4610

Signed-off-by: Navin P S <navinp0304@gmail.com>
Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the 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.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Aug 17, 2024
Copy link

Thank you for signing the OCA.

@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Aug 19, 2024
@navinp0304
Copy link
Contributor Author

@vladak please review this PR

@vladak
Copy link
Member

vladak commented Aug 23, 2024

There should be a test file that actually uses these newly supported properties. The lexer should be run on the file and compare the xref to golden stored output - see https://github.com/oracle/opengrok/tree/master/opengrok-indexer/src/test/resources/analysis/fortran

@navinp0304
Copy link
Contributor Author

There should be a test file that actually uses these newly supported properties. The lexer should be run on the file and compare the xref to golden stored output - see https://github.com/oracle/opengrok/tree/master/opengrok-indexer/src/test/resources/analysis/fortran

Can you give me more info on this one ?

@vladak
Copy link
Member

vladak commented Aug 29, 2024

There should be a test file that actually uses these newly supported properties. The lexer should be run on the file and compare the xref to golden stored output - see https://github.com/oracle/opengrok/tree/master/opengrok-indexer/src/test/resources/analysis/fortran

Can you give me more info on this one ?

The Fortran analysis tests are under https://github.com/oracle/opengrok/tree/master/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/fortran

From there, the

void sampleTest() throws IOException {
writeAndCompare(new FortranAnalyzerFactory(),
"analysis/fortran/sample.f",
"analysis/fortran/sample_xref.html",
readTagsFromResource("analysis/fortran/sampletags"), 28);
}
is the most relevant for this change. It runs the lexer on the https://github.com/oracle/opengrok/blob/master/opengrok-indexer/src/test/resources/analysis/fortran/sample.f file and compares the result with the golden file https://github.com/oracle/opengrok/blob/master/opengrok-indexer/src/test/resources/analysis/fortran/sample_xref.html

For this case, I'd recommend creating new Fortran code under https://github.com/oracle/opengrok/tree/master/opengrok-indexer/src/test/resources/analysis/fortran with its expected output and adding a new test (or better parametrize the pre-existing one referenced above). The golden file (expected result) can be obtained e.g. by running the indexer on the sample file or by extracting the data from debugger when stepping through the newly added test case.

@navinp0304 navinp0304 marked this pull request as draft October 1, 2024 16:27
@navinp0304 navinp0304 requested a review from vladak October 1, 2024 16:27
@navinp0304
Copy link
Contributor Author

@vladak Made the changes suggested by you.

@navinp0304 navinp0304 marked this pull request as ready for review October 2, 2024 03:55
Copy link
Member

@vladak vladak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the onNonSymbolMatched() arguments

Signed-off-by: Navin P <navinp0304@gmail.com>
@navinp0304 navinp0304 requested a review from vladak October 3, 2024 13:43
@navinp0304
Copy link
Contributor Author

navinp0304 commented Oct 3, 2024

fix the onNonSymbolMatched() arguments

You mean this 84c1728 ?

@vladak
Copy link
Member

vladak commented Oct 3, 2024

fix the onNonSymbolMatched() arguments

You mean this 84c1728 ?

yep, something like this (modulo the missing white space around binary operators)

@navinp0304
Copy link
Contributor Author

navinp0304 commented Oct 4, 2024

fix the onNonSymbolMatched() arguments

You mean this 84c1728 ?

yep, something like this (modulo the missing white space around binary operators)
@vladak
Do you have any scripts like indent,style oe clang-format and the settings I can use ?
Or is it just this ?
onNonSymbolMatched(cmatch.substring(cmatch.length()-1,1)

@vladak
Copy link
Member

vladak commented Oct 4, 2024

Do you have any scripts like indent,style oe clang-format and the settings I can use ?

The style check is run during Maven build of the project using the maven-checkstyle-plugin (e.g. mvn -DskipTests=true verify), the rules are stored in https://github.com/oracle/opengrok/tree/master/dev/checkstyle

@navinp0304
Copy link
Contributor Author

navinp0304 commented Oct 4, 2024

Do you have any scripts like indent,style oe clang-format and the settings I can use ?

The style check is run during Maven build of the project using the maven-checkstyle-plugin (e.g. mvn -DskipTests=true verify), the rules are stored in https://github.com/oracle/opengrok/tree/master/dev/checkstyle

Is it run for lex files ? opengrok-indexer/src/main/jflex/analysis/fortran/FortranXref.lex I see no warnings or errors on that file during mvn -DskipTests=true verify

opengrok$ grep -i checkstyle bfile
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ opengrok ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ opengrok ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ plugins ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ plugins ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ suggester ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ suggester ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ opengrok-web ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ opengrok-web ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ tools ---
[INFO] You have 0 Checkstyle violations.
[INFO] --- checkstyle:3.3.1:check (checkstyle) @ tools ---
[INFO] You have 0 Checkstyle violations.

Signed-off-by: Navin P <navinp0304@gmail.com>
@vladak
Copy link
Member

vladak commented Oct 4, 2024

The style check is run during Maven build of the project using the maven-checkstyle-plugin (e.g. mvn -DskipTests=true verify), the rules are stored in https://github.com/oracle/opengrok/tree/master/dev/checkstyle

Is it run for lex files ? opengrok-indexer/src/main/jflex/analysis/fortran/FortranXref.lex I see no warnings or errors on that file during mvn -DskipTests=true verify

Actually, looking at the rules and checkstyle.org documentation I am not sure it can be enforced. Just surround the binary operators with spaces.

@vladak vladak merged commit 2c08f40 into oracle:master Oct 4, 2024
9 checks passed
@vladak
Copy link
Member

vladak commented Oct 4, 2024

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants