dparkins / language-fortran

Syntax highlighting for FORTRAN for atom
MIT License
35 stars 16 forks source link

Add submodule snippet #94

Closed cbcoutinho closed 7 years ago

cbcoutinho commented 7 years ago

Here's a small edit to include a default submodule snippet. I've also noticed that submodules are not correctly highlighted, probably because they are relatively new.

Since I want to learn how to edit grammars, could you explain to me how atom expects to pick up block(s) of code (e.g. module, subroutine, function, etc.). I'm pretty sure it has to do with the regex of a procedure, but I'm no regex expert and am having trouble grokking how this all works.

I'd be happy to keep working on this PR before you merge so that I can learn how atom grammars work.

cbcoutinho commented 7 years ago

Just to expand on my PR: here's what I would like to learn how to change. You see that the module and submodule name after the submodule keyword are not being highlighted correctly. Neither is module procedure.

I think that both the module and submodule name should probably be the same color as, say, a module name. Then again, the program name is just white with my syntax, so it isn't really consistent. Can you explain to me how that works? Does an atom syntax assign text a color based on its meta characteristics or something?

I created a gist if you want to see the code yourself. (link)

screenshot from 2017-02-06 10-20-11

tomedunn commented 7 years ago

Atom assigns colors to text based on the texts scope which are assigned by the language grammar that is applied. Scopes in the syntax rules are the strings that follow the name attribute. So as an example, the if keyword in this language package is assigned the scope keyword.control.if.fortran.

That said, how various scopes are colored is entirely dependent on the color scheme being used. Back from the old Textmate and Sublime Text days there were certain scopes that were recommended as default scopes which are generally included in all color schemes (you can find a brief overview here). For example the scope entity.name.function was used more or less as a default scope. This is why the scope for a subroutine name is entity.name.function.subroutine.fortran rather than entity.name.subroutine.fortran, in order to keep the color assigned to function and subroutine names the same.

For modules the current scope applied to the module name is entity.name.module.fortran and for submodules it is entity.name.submodule.fortran. I left it this way because neither were part of the default set of scopes so having a specific color assigned to them might not be universally supported (hence why entity.name.program.fortran isn't colored in your code). However, I agree that it would be nice for the two to have the same coloring regardless of the color scheme being used. So it would probably be best to change the scope of the submodule name to entity.name.module.submodule.fortran.

As for why the module name in the submodule definition isn't being highlighted, that's simply because I never added any rules for that part of the code. :smile:

tomedunn commented 7 years ago

@cbcoutinho are you sure your module procedure example is valid? I'm not familiar with it and I'm having a hard time finding documentation for it. So far I've only been able to find documentation for module procedure statements of the following form:

module procedure :: name1, name2 

I'm also not finding anything for it in the 2008 standard. Is this from a newer standard?

cbcoutinho commented 7 years ago

Wonderful, thanks for the great explanation.

I added a procedure definition, which correctly picks up module procedure, by copying and slightly adjusting the subroutine definition. Dummy variables are invalid for module procedures, so I just set remaining characters as invalid.

I have two questions:

  1. I don't know how to make the term module required: module procedures need to have the keyword module in the beginning, unlike module functions and module subroutines where it's optional. How can you set this as invalid code?

screenshot from 2017-02-06 13-42-22

  1. What is the difference in $base used in the modern grammar and $self used in free form?
cbcoutinho commented 7 years ago

Yes I'm sure it's valid. I am using module procedures defined in submodules, whereas the interfaces are defined in their 'base' module. See this example on the fortran wiki.

That's why I wanted to be more specific in what's valid and what isn't

tomedunn commented 7 years ago

OK, I was able to find reference to your module procedure statement in the copy of the 2008 standard that I have (section 12.6.2.5).

To make the rule require module procedure in it's entirety change the initial rule to

'procedure-definition':
    'comment': 'Procedure program unit. Introduced in the Fortran 2008 standard.'
    'name': 'meta.procedure.fortran'
    'begin': '(?i)(?:^|(?<=;))(?=[^\'";!\\n]*\\bmodule\\s+procedure\\b)'
    'end': '(?=[;!\\n])'
    'patterns':[
      {
        'begin': '(?i)\\s*\\b(module\\s+procedure)\\b'
        'beginCaptures':
          '1': 'name': 'keyword.other.procedure.fortran'
        'end': '(?=[;!\\n])'
        'patterns':[
          {
            'comment': 'Procedure body.'
            'begin': '(?i)\\G\\s*\\b([a-z]\\w*)\\b'
            'beginCaptures':
              '1': 'name': 'entity.name.function.procedure.fortran'
            'end': '(?ix)\\s*\\b(?:(end\\s*procedure)(?:\\s+(\\1))?|(end))\\b
              \\s*([^;!\\n]+)?(?=[;!\\n])'
            'endCaptures':
              '1': 'name': 'keyword.other.endprocedure.fortran'
              '2': 'name': 'entity.name.function.procedure.fortran'
              '3': 'name': 'keyword.other.endprocedure.fortran'
              '4': 'name': 'invalid.error.fortran'
            'patterns': [
              {
                'comment': 'Rest of the first line in procedure construct - should be empty.'
                'name': 'meta.first-line.fortran'
                'begin': '\\G(?!\\s*[;!\\n])'
                'end': '(?=[;!\\n])'
                'patterns':[
                  {'include': '#invalid-character'}
                ]
              }
              {
                'comment': 'Specification and execution block.'
                'name': 'meta.block.specification.fortran'
                'begin': '(?i)(?!\\s*(?:contains\\b|end\\s*[;!\\n]|end\\s*procedure\\b))'
                'end': '(?i)(?=\\s*(?:contains\\b|end\\s*[;!\\n]|end\\s*procedure\\b))'
                'patterns':[
                  {'include': '$self'}
                ]
              }
              {
                'comment': 'Contains block.'
                'name': 'meta.block.contains.fortran'
                'begin': '(?i)\\s*(contains)\\b'
                'beginCaptures':
                  '1': 'name': 'keyword.control.contains.fortran'
                'end': '(?i)(?=\\s*(?:end\\s*[;!\\n]|end\\s*procedure\\b))'
                'patterns':[
                  {'include': '$self'}
                ]
              }
            ]
          }
        ]
      }
    ]

Hopefully that works. Sometimes you run into issues where the order of rules as they are listed in the patterns section matter. So if the rule doesn't work at first try to disable other rules in the patterns section to see if you can get it to. If you can get it to work that way then it's just a matter of positioning the rule in the right order with the other rules. If it doesn't then I might have made a mistake in how I altered it.

tomedunn commented 7 years ago

@cbcoutinho the difference between $base and $self is pretty subtle (and I may not remember it perfectly). From what I recall $base will pull rules from whatever language the current grammar is embedded in while $self will only pull rules from the grammar the current structure comes from. In practice it doesn't make much of a difference. I think my rational for using $base instead of $self was that I thought it might come up when importing the fortran - free form rules into fortran - fixed form but I think the way I did it with grammar injections made that irrelevant.

tomedunn commented 7 years ago

Looks like I remembered it correctly! :smile: You can find some discussion of the difference between $base and $self here.

cbcoutinho commented 7 years ago

Sounds great @tomedunn.

I applied the changes you mentioned, namely getting rid of the attributes prefixing module procedure. I left $self and $base as is

cbcoutinho commented 7 years ago

Hmm, that's an interesting subtlety. I guess that could be useful if you mix languages together within the same block - templating comes to mind.

Anyways, I think I'm done here, feel free to merge if you think it meets your requirements.

tomedunn commented 7 years ago

@cbcoutinho, thanks for the work on this! Everything looks good at a passing glance (I'm guessing the changes work for your test cases). I should have time to test out these changes later on today just to make sure nothing unexpected breaks in any of my test files.

Also, if you choose to make any future changes don't feel the need to apply them to the Fortran - Modern rules. I'm planning on getting rid of Fortran - Modern and Fortran - Punchcard at some point in the future as they have been replaced by Fortran - Fixed Form and Fortran - Free Form.

cbcoutinho commented 7 years ago

No problem, something I use always has my attention for some TLC.

One more fix, indentation wasn't being done right. Can you check the latest commit when you get a chance?

cbcoutinho commented 7 years ago

You mean like this? The first part I understood, but the second request I didn't quite understand. I just moved everything in that block up a level.

cbcoutinho commented 7 years ago

Oops, nope. Didn't test - everything's broken. I didn't understand what you meant by your second point

tomedunn commented 7 years ago

That's alright. The second point would just remove some redundancy in the code. The first point is the only one that fixes an actual problem.