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

Request: (fortran) Add support for FORTRAN 77 style comments #2310

Closed
interkosmos opened this issue Dec 4, 2019 · 27 comments · Fixed by #2416
Closed

Request: (fortran) Add support for FORTRAN 77 style comments #2310

interkosmos opened this issue Dec 4, 2019 · 27 comments · Fixed by #2416
Labels
enhancement An enhancement or new feature good first issue Should be easier for first time contributors help welcome Could use help from community language

Comments

@interkosmos
Copy link
Contributor

The style for source code in Fortran accepts only ! as a comment identifier, not the old C as in FORTRAN 77 and earlier. Is it possible to treat C as the beginning of a comment?

@joshgoebel
Copy link
Member

Please show us some example code.

@joshgoebel joshgoebel changed the title Adding support for FORTRAN 77 comments Request: (fortran) Add support for FORTRAN 77 style comments Dec 4, 2019
@interkosmos
Copy link
Contributor Author

interkosmos commented Dec 4, 2019

Please see this example:

<pre><code class="fortran">
C
C  This program in FORTRAN 77 outputs "Hello, World!".
C
      PROGRAM MAIN
      WRITE (*, *) 'Hello, World!'
      END
</code></pre>

The C is not recognised as a comment, while ! for Fortran ≥ 90 is.

@joshgoebel
Copy link
Member

Can a line in Fortran >= 90 not legitimately start with C?

@interkosmos
Copy link
Contributor Author

Fortran 90 introduced free-format that requires to use ! for comments. Only in the old fixed source format the C comment style is still allowed. That means, on Fortran 90, it depends on the file ending/the used compiler flag.

@joshgoebel
Copy link
Member

It sounds like we can really only support one then since we can’t know if the file is FORTRAN 90 or FORTRAN 77 - so we can’t know which rule to apply. And having two separate FORTRAN languages seems a bit much. Am I missing something?

@joshgoebel
Copy link
Member

Fortran 90 introduced free-format that requires to use ! for comments. Only in the old fixed source format the C comment style is still allowed.

I'm not sure you understood. I meant in Fortran 90 can C meaning something else entirely? Like the name of a user defined function, etc... then it's meaning would be ambiguous if you didn't have the compiler flag or file ending, so it'd be impossible for us to know whether the source in question was Fortran 77 or Fortran 90.

@interkosmos
Copy link
Contributor Author

I'm not sure you understood. I meant in Fortran 90 can C meaning something else entirely? Like the name of a user defined function, etc... then it's meaning would be ambiguous if you didn't have the compiler flag or file ending, so it'd be impossible for us to know whether the source in question was Fortran 77 or Fortran 90.

Fortran is quite backwards compatible. That means, C just indicates a comment and nothing else, even if modern Fortran compiler will complain about the incorrect comment. But there is some ambiguity related to variables and functions, i. e.:

C This is a comment. Now, we assign foo to variable c:
c = foo

@joshgoebel
Copy link
Member

Ah, yes since case_insensitive is true then yes, this is one example of the problem I was referring to...

[c/C] This is a comment. Now, we assign foo to variable c:
[c/C] = foo

No solid way to tell a comment from something else.

@interkosmos
Copy link
Contributor Author

My example is misleading. Of course, writting in FORTRAN 77 requires fixed format, that means the first six characters of each line are reserved for line number, format descriptions, carriage control, and so on (no statements allowed). But that would require to distinguish between FORTRAN 77 and Fortran 90.

FORTRAN 77 fixed format:

C This is a FORTRAN 77 example.
      PROGRAM MAIN
      PRINT *, 'Hello!'
      END

Fortran 90 free format:

! This is a Fortran 90 example.
program main
    print *, 'Hello!'
end program main

The Wikipedia linter seems to get it right.

@joshgoebel
Copy link
Member

But that would require to distinguish between FORTRAN 77 and Fortran 90.

Right, that's the real problem.

The Wikipedia linter seems to get it right.

An interesting but not necessarily related data-point. ;-)

@joshgoebel joshgoebel added language enhancement An enhancement or new feature labels Dec 6, 2019
@egor-rogov
Copy link
Collaborator

Hm, this is another example of problem that can be solved by parametrized grammar. So that one using Olde Fortran can just turn on C-comments.
Can we achieve it by using callbacks (#2261) to call a function provided by plugin?

@joshgoebel
Copy link
Member

joshgoebel commented Dec 7, 2019

Hm, this is another example of problem that can be solved by parametrized grammar.

When and where are you imagining you'd pass these params to the grammar? I can see that helping SOME people... but I think a lot of people just want to throw two code snippets all the wall - one of then Fortran 77 and the other Fortran 90 and it magically just works.

<pre><code data-hljs-language="fortran(version:77)">...

??? Or were you imagining GLOBALLY setting such things? That's already SORTA possible if you hacked registerLanguage a bit...

registerLanguage("fortran", fortranLangDefinition, {version: 77})

Can we achieve it by using callbacks (#2261) to call a function provided by plugin?

The conditional rules perhaps... but not passing the params themselves, that's something new (one idea above).

@joshgoebel
Copy link
Member

joshgoebel commented Dec 7, 2019

@interkosmos Would specifying the variant (77 or 90) manually help you, or were you really just wanting it to work automagically?

@egor-rogov
Copy link
Collaborator

Of course everybody will prefer automagic. But looks like here we just can't do it, right?

I imagined something like a rule for C-comments in grammar which tries to get a parameter value from a plug-in that handles some special event. If no plug-in registered, no parameters received.

@joshgoebel
Copy link
Member

joshgoebel commented Dec 7, 2019

Passing parameters sounds like a potentially useful feature for core to me... I could imagine how you'd frankenstein it on top of plugin/callback functionality we don't have yet... but if we thought it was a good idea I think it'd be easier to support in the core lib.

Having to write a plugin for the sole purpose of parametrizing a language sounds blah to me.

You could also just define a configureLanguage API that attaches an object to the language... and then that data would be available to callbacks:

configureLanguage("fortran",{version:77})
// ...
paramDefaults: {version: 90 }
// ...
{
skipThisRuleIf: (x, params) => {params.version == 77 }
...
skipThisRuleIf: (x, params) => {params.version == 90 }
}

@joshgoebel
Copy link
Member

Although skipping entire rules dynamically starts to get complex since we aggregate the rules when we compile the grammar... but that's a technical problem... obviously it's easy to imagine in theory some simple of if/unless callbacks (as shown above).

@interkosmos
Copy link
Contributor Author

@interkosmos Would specifying the variant (77 or 90) manually help you, or were you really just wanting it to work automagically?

It would be totally fine to select the language version.

@joshgoebel
Copy link
Member

Per code section or is your whole usage all one version of FORTRAN or the other?

@interkosmos
Copy link
Contributor Author

Per code section would be great, as this would make it possible to compare language versions.

@joshgoebel
Copy link
Member

This also complicates testing since right now there is no way to configure detects/markup tests to pass this extra info. For example we'd really want to test both the Fortran 77 and 90 comment functionality with two different markup tests to make sure both config options work.

@joshgoebel
Copy link
Member

joshgoebel commented Dec 26, 2019

But that would require to distinguish between FORTRAN 77 and Fortran 90.

At this point it's starting to sound like a separate grammar since then you could check for the spacing and give it relevancy so then you'd "detect" whether a fortran file was 77 or 90 version. I've also suggested the idea of "rollup" languages elsewhere.

So that fortran => { fortran77, fortran90}... and then if you said code was "fortran" it would highlight with whichever seems to match best.

@joshgoebel joshgoebel added the help welcome Could use help from community label Feb 7, 2020
@joshgoebel
Copy link
Member

joshgoebel commented Feb 18, 2020

[c/C] This is a comment. Now, we assign foo to variable c:
[c/C] = foo

Can you think of other false positive scenarios?

If we simply added a "C" <whitespace> <anything but "="> rule that was a comment, would this work but not break any valid Fortran 90 code? It would prevent "pretty comments" like:

C =================
C This is a pretty comment with borders...
C ==================

Does Fortran have a == operator?

@interkosmos
Copy link
Contributor Author

interkosmos commented Feb 19, 2020

Can you think of other false positive scenarios?

I was looking at arbitrary source code, but couldn’t find any other false positives.

Does Fortran have a == operator?

Yes, but only inside statements (e.g., if, do, where, forall, …).

@joshgoebel
Copy link
Member

So if we added the following as comment patterns:

  • ^C ==.* to allow ===== runs
  • ^C [^=].* to prevent assignments from being mistaken as comments

Would that get the job done?

@interkosmos
Copy link
Contributor Author

The white-space between ^C and = is optional in Fortran.

@joshgoebel
Copy link
Member

Yeah, I figured... the final regex would allow for whitespace variance, but otherwise does that look good?

@interkosmos
Copy link
Contributor Author

Should work, so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature good first issue Should be easier for first time contributors help welcome Could use help from community language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants