opengrok icon indicating copy to clipboard operation
opengrok copied to clipboard

Add support for $ in fortran identifiers and clickable .i include files.

Open navinp0304 opened this issue 1 year ago • 5 comments

Implementation according to discussion based on vladak https://github.com/oracle/opengrok/issues/4610 I've verified that the include 'file' are clickable. I've signed the oca.

navinp0304 avatar Aug 17 '24 12:08 navinp0304

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.

Thank you for signing the OCA.

@vladak please review this PR

navinp0304 avatar Aug 19 '24 14:08 navinp0304

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

vladak avatar Aug 23 '24 15:08 vladak

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 ?

navinp0304 avatar Aug 28 '24 15:08 navinp0304

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 https://github.com/oracle/opengrok/blob/f79db409d75bcf91ceab9ed0e9dcbfcde691d838/opengrok-indexer/src/test/java/org/opengrok/indexer/analysis/fortran/FortranXrefTest.java#L39-L44 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.

vladak avatar Aug 29 '24 09:08 vladak

@vladak Made the changes suggested by you.

navinp0304 avatar Oct 01 '24 16:10 navinp0304

fix the onNonSymbolMatched() arguments

You mean this https://github.com/oracle/opengrok/pull/4642/commits/84c1728e7fd88ffa5d2df17957c54558257b9c7d ?

navinp0304 avatar Oct 03 '24 13:10 navinp0304

fix the onNonSymbolMatched() arguments

You mean this 84c1728 ?

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

vladak avatar Oct 03 '24 14:10 vladak

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)

navinp0304 avatar Oct 04 '24 12:10 navinp0304

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

vladak avatar Oct 04 '24 13:10 vladak

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.

navinp0304 avatar Oct 04 '24 13:10 navinp0304

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 avatar Oct 04 '24 14:10 vladak

Thanks !

vladak avatar Oct 04 '24 16:10 vladak