Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Erroneous syntax highlighting, when argument is called "function" or "procedure" #207

Closed
gnikit opened this issue Dec 11, 2020 · 6 comments · Fixed by #244 or #445
Closed

Erroneous syntax highlighting, when argument is called "function" or "procedure" #207

gnikit opened this issue Dec 11, 2020 · 6 comments · Fixed by #244 or #445

Comments

@gnikit
Copy link
Member

gnikit commented Dec 11, 2020

This is another edge case that the TextMate bundle did not account for. Some keywords should be escaped as when not matching their normal regex pattern.

image

  interface set_scalar_field_from_python
    module procedure set_scalar_field_from_python_sp

    subroutine set_scalar_field_from_python(function, function_len, dim, &
         nodes, x, y, z, t, result, stat)
      use iso_c_binding, only: c_double, c_int, c_char
      implicit none
      integer(c_int), intent(in), value :: function_len
      character(kind=c_char, len=function_len) :: function
      integer(c_int), intent(in), value :: dim, nodes
      real(c_double), dimension(nodes), intent(in) :: x, y, z
      real(c_double), intent(in), value :: t
      real(c_double), dimension(nodes), intent(out) :: result
      integer(c_int), intent(out) :: stat
    end subroutine set_scalar_field_from_python
  end interface set_scalar_field_from_python
@pedro-ricardo
Copy link
Collaborator

Hello, please check if adding the ^\\s* in the begin variable in function-definition scope as below solves your issue

In here

"function-definition": {
    "comment": "Funtion program unit. Introduced in the Fortran 1977 standard.",
    "name": "meta.function.fortran",
    "begin": "(?i)(?=([^'\";!\\n](?!\\bend))*\\bfunction\\b)",
    "end": "(?=[;!\\n])",

Change

    "begin": "(?i)(?=([^'\";!\\n](?!\\bend))*\\bfunction\\b)",

To

    "begin": "(?i)(?=([^'\";!\\n](?!\\bend))*^\\s*\\bfunction\\b)",

If it works, please consider adding the same fix to subroutine-definition in the same PR.

@gnikit
Copy link
Member Author

gnikit commented Feb 6, 2021

@pedro-ricardo Do you know if we can somehow unittest the syntax highlighting? I fear that in fixing one thing we might break another

@pedro-ricardo
Copy link
Collaborator

When debugging the highlights I use the editor.action.inspectTMScopes to check if the word is in the correct scope and if not why it ended in a different scope.
So I suppose it is possible to create an example file, just like the one used in the symbol check, that will verify if the words are in the scope they should.

But since I've only changed the regex stuff in the extension so far, I have no knowledge of how the typescript files work inside it.

Maybe you should create another issue with this feature request. When this background of the test is done I imagine that it will be easier to add more and more cases for it to check.

@gnikit
Copy link
Member Author

gnikit commented Feb 14, 2021

Hello, please check if adding the ^\\s* in the begin variable in function-definition scope as below solves your issue

In here

"function-definition": {
    "comment": "Funtion program unit. Introduced in the Fortran 1977 standard.",
    "name": "meta.function.fortran",
    "begin": "(?i)(?=([^'\";!\\n](?!\\bend))*\\bfunction\\b)",
    "end": "(?=[;!\\n])",

Change

    "begin": "(?i)(?=([^'\";!\\n](?!\\bend))*\\bfunction\\b)",

To

    "begin": "(?i)(?=([^'\";!\\n](?!\\bend))*^\\s*\\bfunction\\b)",

If it works, please consider adding the same fix to subroutine-definition in the same PR.

I think I might need to do some more work at escaping keywords because character(kind=c_char, len=function_len) :: function still seems to be wrong. I will have a look.

@gnikit
Copy link
Member Author

gnikit commented Oct 8, 2021

So this is actually way more annoying than it looks.
We need to escape keyword.other.function.fortran scope for when we have :: or ( + any and all whitespaces that follow.
so something like this (?<!::\\s*) should cover the ::, however, lookbehinds need to be of fixed width so theoretically we can't use *. It would appear that the VSCode regex can handle variable width negative look-behinds, so this fixes the :: issue.

We are still left with subroutine name( function ). That one gets highlighted red by meta.attribute-list.fortran. We need to make the begin expression more greedy. We could achieve this by doing a no capture on ( and all whitespaces that follow.

"begin": "(?i)(?=([^'\";!\\n](?!\\bend))*(?:(?<!\\(\\s*))\\bfunction\\b)",

This is actually a lot more work since we should technically be able to escape all of Fortran keywords e.g. function, subroutine, module, submodule, procedure, contains,...

Also, the solution I have created does not solve the problem the right way. Instead of placing the variables in meta.parameter.fortran it places them in meta.function.fortran. Thankfully the input arguments in the input arg list are captured correctly in variable.parameter.fortran

@gnikit
Copy link
Member Author

gnikit commented Oct 11, 2021

This has now been fixed. The solution was a bit more involved than what I originally stated but should be okay now.

gnikit added a commit that referenced this issue Oct 11, 2021
…n" or "procedure" #207

I have used non-fixed width ngative lookbehinds which should not
 be permitted in traditional Oniguruma regex. VSCode's implementation of
 the regex engine does not seem to care hence the solution
@gnikit gnikit mentioned this issue Oct 11, 2021
19 tasks
krvajal pushed a commit that referenced this issue Oct 20, 2021
* Updated CHANGELOG for v2.2.2

* Incremented package version to 2.2.2

* Minor aesthetic improvements to CHANGELOG

* Adds OpenACC unit test

* Updated tasks and launch files

- The file structures have been updated to abide with the latest config syntax
- Updated build commands in package.json
- Deleted tasks.json.old

* Fixes Remove unused packages #243

Regenerated npm and yarn .json files

* Updated README badges

* Updated workflows to run on Ubuntu latest

* Fix overzealous OpenMP regex.

The OpenMP regex did not allow for the OpenACC syntax scope to trigger.
This has now been fixed and both should be displayed correctly.

The only outstanding problem is that the unittest (.snap) does not
trigger the right scopes, which means open acc/openmp are not
tested thoroughly. I look into it

* Updating changelog.md

* New minor release

* Housekeeping

Changes all fortls instances with global variable
and makes pip install user based with upgrade.

* Added info to package.json

* Fixes in-house documentaiton hover

Fixes #250

* Now the deocumentation displays correctly

Having preceding characters to ``` caused a problem in the hover result

* Formatting .json doc files with prettier

* Updated README.md

* Adds VS marketplace automated release

Fixes Setting up VSCE releases from GitHub releases #237

* Further improvements to the hover documentation

* further fixes for internal documentation

* Adds autoclosing for strings

* Updates Fortran extensino and adds .pFUnit support

Fixes #185.

* Updates CHANGELOG.md

* Fixes preprocessor syntax highlighting

The line continuation operator is a bit too aggressive so instead of
adding lookaheads for every case where we don't need to apply it
we have excluded the preprocessor directives from the lint cont.

The original .cson highlighting does the same, see:
https://github.com/dparkins/language-fortran/blob/master/grammars/fortran%20-%20free%20form.cson

for injections see:
https://gist.github.com/Aerijo/b8c82d647db783187804e86fa0a604a1

Fixes Preprocessor statements in line continuations break syntax highlighting #248
Fixes Erroneous syntax highlighting for preprocessor conditionals in derived types #249

* Preprocessor assignment i.e. = is not a thing

The regex was doing a negative look ahead and lookbehind for =
but using = is illegal code and will not compile. e.g. #define VAR = 1

Also I went ahead and changed the patterns #define can match to be
both string literals and numerical values #define VAR 1 is legal.

* Preprocessor operator fixes

- Adds support for all Fortran supported logical preprocessor operators
- Adds support for arithmetic operators
- Adds support for C++ preprocessor integers
- Changes the syntax highlighting of preprocessor commands to use the
   `meta` scope which should result in consistent coloring between
   C++ and Fortran. A few things are not supported like macro function
   argument highlighting but I do not believe it is important for now

* Fixes Erroneous syntax highlighting, when argument is called "function" or "procedure" #207

I have used non-fixed width ngative lookbehinds which should not
 be permitted in traditional Oniguruma regex. VSCode's implementation of
 the regex engine does not seem to care hence the solution

* Adds unittest for #207 and updates CHANGELOG

* Adds syntax highlighting support for fypp
also extends the support for pfunit.

Not sure if .pf and .fpp are considered to be fixed-form by default
I don't think that is the case but fypp and pfunit use them so we
default them to free-form

* Add MIT license badge back to README

* Adds names specific to individual scopes

This is meant to make debugging syntax highlighting bugs easier to trace

The unittests are also updated to contain the new scope names.

* Fixes Erroneous syntax highlighting of if construct with tags #204

Labels were only captured at the start and end of a statement.
Now we are also capturing them in between for if conditionals.

 The edits in the end in "named-control-constructs" are meant to
 correctly handle whitespaces which before they were placed as part of
 the group returned to the invalid.error.xxx

 A unittest has been added testing the conditionals with/out labels.

* Updated CHANGELOG.md

* Fixes STOP named_string #172

`stop` can now handle labels

* Comments are correctly highlighted for type,...

Fixes Erroneous syntax highlighting with type,abstract :: var #262
A unittest has been added and the CHANGELOG has been updated.

* Add syntax test for fixed form fortran

* Switches to @types/vscode & @vscode/test-electron

Also updates the tests to use strictEqual
Adds production, test and dev tscofig compilation

Fixes Migrate from vscode module #263

* Updated changelog

* Updated yarn.lock

* Increments version to 2.4.0

Release a tag after token is uploaded

* Updated tasks.json and launch.json

External extensions are enabled since we need the C++ extension
for VSCode to launch without throwing an error.
The tasks.json has been updated to call directly scripts from package.

* Updated names of scopes to contain fortran

* Fixes normal labeled construct end statements

* Adds error highlighting for else labeled

* Upgraded package.json grammar update
@gnikit gnikit reopened this Apr 27, 2022
gnikit added a commit that referenced this issue Apr 27, 2022
Fixes #309

Fixes again #207 but without using non-fixed width look behinds
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
2 participants