Minor hotfixes #53

pedro-ricardo · 2018-04-18T11:08:59Z

Solves issue #51 by making module end use names with the same colors

codecov · 2018-04-18T11:14:19Z

Codecov Report

Merging #53 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #53   +/-   ##
=======================================
  Coverage   13.21%   13.21%           
=======================================
  Files           9        9           
  Lines         348      348           
  Branches       49       49           
=======================================
  Hits           46       46           
  Misses        302      302

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b50a2b4...6fc6737. Read the comment docs.

krvajal

@pedro-ricardo could you please take a look at my comments and update your PR.

krvajal · 2018-04-18T20:37:49Z

syntaxes/fortran_free-form.tmLanguage.json

@@ -3631,7 +3631,7 @@
 					"begin": "(?i)\\s*\\b([a-z]\\w*)\\b",
 					"beginCaptures": {
 						"1": {
-							"name": "entity.name.module.fortran"


Why the change of name. What is being captured here is the name of the module, not of a class.

It is because inside "use-statement": rule, on line 4427 it is written "name": "entity.name.class.fortran" this means that use will highlight the following name as class. And as issue #51 requested, module should highlight the following name that same way as use does.
But It could be done the other way around. To just correct the "use-statement": rule to highlight the following name as module instead of class.
I chose the previous because entity.name.module.fortran has the same color as non-highlighted text.

krvajal · 2018-04-18T20:38:03Z

syntaxes/fortran_free-form.tmLanguage.json

@@ -3640,7 +3640,7 @@
 							"name": "keyword.other.endmodule.fortran"
 						},
 						"2": {
-							"name": "entity.name.module.fortran"
+							"name": "entity.name.class.fortran"


Sames as previous comment

krvajal · 2018-04-24T16:52:20Z

Well in that case there should be a way to change the color. The color is wrong, the naming is not. Get Outlook for Android On Tue, Apr 24, 2018 at 5:55 PM +0200, "Pedro Ricardo C Souza" <[email protected]> wrote: @pedro-ricardo commented on this pull request. In syntaxes/fortran_free-form.tmLanguage.json:

@@ -3631,7 +3631,7 @@

"begin": "(?i)\\s*\\b([a-z]\\w*)\\b", "beginCaptures": { "1": { - "name": "entity.name.module.fortran" It is because inside "use-statement": rule, on line 4427 it is written "name": "entity.name.class.fortran" this means that use will highlight the following name as class. And as issue #51 requested, module should highlight the following name that same way as use does. But It could be done the other way around. To just correct the "use-statement": rule to highlight the following name as module instead of class. I chose the previous because entity.name.module.fortran has the same color as non-highlighted text. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

pedro-ricardo · 2018-04-24T17:16:06Z

Well, the "use-statement": name is wrong, it should be entity.name.module.fortran not class as it is. I'll look how to add a color to this since it does not exist in VSCode themes.

pedro-ricardo · 2018-04-24T19:26:08Z

I've been looking at this issue and the problem is that entity.name.module entry does not exist inside default VSCode themes, but entity.name.class does.

Example of Dark+ theme

		{
		"name": "Types declaration and references",
		"scope": [
(...)
	  	"entity.name.type",
		"entity.name.class",
(...)
		],
		"settings": {
		"foreground": "#4EC9B0"
		}

I don't think that customizing the theme is a good option ...

krvajal · 2018-04-26T08:14:52Z

I am not suggesting you change the colour itself to a specific colour. If I understand correctly your objective is to make those keywords to have the same colour (no matter what that colour is specifically) so there should be a mechanism for that. But changing the name that identifies the syntactical meaning of the token to something else does not seems like the right thing to do.

krvajal · 2018-04-26T08:20:40Z

So what you can see from the images below the scope defined by the module name is not been highlighted because there is no such scope defined in the current theme, but the one in the use statement does get highlighted because in this case there is a scope defined in the theme. As we dot provide a theme of our own. We have to rely on the common scopes that for sure will be included in any decent theme.
Looking into the list of available scopes (here for example). It seems to me that the scope name that makes more sense, in this case, is entity.name.namespace. So both of the instances should match the same scope and then they will get highlighted with the same colour.

It would be nice if you can update your PR changing this and then we can merge 😀

pedro-ricardo · 2018-04-26T12:10:44Z

I agree completely ... but although this entry entity.name.namespace is a valid scope naming, it still has no theme selector in VSCode.
Below i list the results of grep -Rin "entity.name" inside VSCode theme-defaults folder.

themes/dark_plus.json:9:                                "entity.name.function",
themes/dark_plus.json:23:                               "entity.name.type",
themes/dark_plus.json:24:                               "entity.name.class",
themes/dark_vs.json:60:                 "scope": "entity.name.tag",
themes/dark_vs.json:66:                 "scope": "entity.name.tag.css",
themes/light_vs.json:58:                        "scope": "entity.name.tag",
themes/light_vs.json:64:                        "scope": "entity.name.selector",
themes/hc_black_defaults.json:81:                       "scope": "entity.name.tag",
themes/hc_black_defaults.json:87:                       "scope": "entity.name.tag.css",
themes/hc_black.json:13:                                "entity.name.function",
themes/hc_black.json:27:                                "entity.name.type",
themes/hc_black.json:28:                                "entity.name.class",
themes/light_plus.json:9:                               "entity.name.function",
themes/light_plus.json:23:                              "entity.name.type",
themes/light_plus.json:24:                              "entity.name.class",

As you can see, there is no entity.name.namespace list there. Therefore inside VSCode it will be the same as entity.name.module or simply no highlight (I've already tested).

On default Dark+ theme, our extension(master branch) uses the following:

entity.name.function is yellowish and used to mark the name followed by both function and subroutine keywords.
entity.name.type is green and used to mark the name followed by both type and class declarations
entity.name.class is green and used to mark the name followed by keyword use
entity.name.tag is blue and isn't used

To follow your thought (which i completely agree) that:

We have to rely on the common scopes that for sure will be included in any decent theme.

I'm afraid those 4 above are our available common scopes

pedro-ricardo · 2018-04-26T13:03:28Z

Just found another one:

entity.other.attribute-name is light-blue and currently not used

Seems like a generic scope name that makes more sense. Do you agree with this one for use and module names?

krvajal · 2018-04-26T18:42:20Z

@pedro-ricardo I personally don't like that one very much as it is too generic. If you find a better scope name that can be used and does not have another meaning in Fortran then go for it. If you can't find any then I won't complain any longer

Regarding the entity.name.tag that you say is not used. I make me wonder if we are highlighting Fortran tags somehow?

krvajal · 2018-04-26T18:52:31Z

If this helps,
find below the basic scopes you can find in the monokia theme

	"scope": [
			"scope": "comment",
			"scope": "string",
			"scope": [
			"scope": [
			"scope": "constant.numeric",
			"scope": "constant.language",
			"scope": "constant.character, constant.other",
			"scope": "variable",
			"scope": "keyword",
			"scope": "storage",
			"scope": "storage.type",
			"scope": "entity.name.type, entity.name.class",
			"scope": "entity.other.inherited-class",
			"scope": "entity.name.function",
			"scope": "variable.parameter",
			"scope": "entity.name.tag",
			"scope": "entity.other.attribute-name",
			"scope": "support.function",
			"scope": "support.constant",
			"scope": "support.type, support.class",
			"scope": "support.other.variable",
			"scope": "invalid",
			"scope": "invalid.deprecated",
			"scope": "meta.structure.dictionary.json string.quoted.double.json",
			"scope": "meta.diff, meta.diff.header",
			"scope": "markup.deleted",
			"scope": "markup.inserted",
			"scope": "markup.changed",
			"scope": "constant.numeric.line-number.find-in-files - match",
			"scope": "entity.name.filename.find-in-files",
			"scope": "markup.quote",
			"scope": "markup.list",
			"scope": "markup.bold, markup.italic",
			"scope": "markup.inline.raw",
			"scope": "markup.heading",
			"scope": "markup.heading.setext",
			"scope": "token.info-token",
			"scope": "token.warn-token",
			"scope": "token.error-token",
			"scope": "token.debug-token",
			"scope": "variable.language",

pedro-ricardo · 2018-04-28T20:45:34Z

In my opinion we should use the logic that's already implemented in the syntax file. And here is an example:
Function names are: entity.name.function.fortran
Subroutine names are: entity.name.function.subroutine.fortran
In this example we use entity.name.function because it is the scope we have and typify with subroutine.fortran that doesn't contribute to to the highlight but make it less generic.

Thinking that this scopes where not made for Fortran language, and that the C++ or Python Classes are the closest features from a Fortran module, I don't have any problem in using the scope entity.name.class and typify with module.fortran. Or use the previous entity.other.attribute-name and add module.fortran.

In the case you are still worried about the confusion with Fortran classes ...
Fortran classes are just a special derived type, and are highlighted as such, for example:
Derived Types declarations are: storage.type.type.fortran
Class declarations are: storage.type.class.fortran
The names of both types and classes are: entity.name.type.fortran

OK so ... to END this once and for all, either choose one

entity.name.class.module.fortran
entity.other.attribute-name.module.fortran

or suggest a new one and I'll put it there. Let's move on 👍 😃

krvajal · 2018-04-28T20:49:48Z

Fill free to use the one that makes more sense to you. And go for it.

…

On Sat, Apr 28, 2018 at 10:45 PM Pedro Ricardo C Souza < ***@***.***> wrote: In my opinion we should use the logic that's already implemented in the syntax file. And here is an example: Function names are: entity.name.function.fortran Subroutine names are: entity.name.function.subroutine.fortran In this example we use entity.name.function because it is the scope we have and typify with subroutine.fortran that doesn't contribute to to the highlight but make it less generic. Thinking that this scopes where not made for Fortran language, and that the C++ or Python Classes are the closest features from a Fortran module, I don't have any problem in using the scope entity.name.class and typify with module.fortran. Or use the previous entity.other.attribute-name and add module.fortran. In the case you are still worried about the confusion with Fortran classes ... Fortran classes are just a special derived type, and are highlighted as such, for example: Derived Types declarations are: storage.type.type.fortran Class declarations are: storage.type.class.fortran The names of both types and classes are: entity.name.type.fortran OK so ... to END this once and for all, either choose one - entity.name.class.module.fortran - entity.other.attribute-name.module.fortran or suggest a new one and I'll put it there. Let's move on 👍 😃 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#53 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFoEefXxpIR1XZhlHyDEaRrAoaaXeLzuks5ttNTugaJpZM4TZ3lh> .

pedro-ricardo · 2018-04-29T01:59:14Z

This PR fixes #57 as well

krvajal · 2018-04-29T02:34:32Z

please do it in a separate pull request as this is not related to the current issue we are fixing.

krvajal · 2018-04-28T23:02:05Z

language-configuration.json

@@ -49,7 +49,8 @@
        ]
    ],
    "indentationRules": {


Pretty cool you are addressing this issue as well, but please do it in a separate pull request as this is not related to the current issue we are fixing.
So just remove this commit from the history.
If you need help doing that ping me and I will let you know how.
After that, we can merge this thing once and for all.

uniform module and use names with the same color.

8387eec

pedro-ricardo mentioned this pull request Apr 18, 2018

Highlighting of module names #51

Closed

krvajal requested changes Apr 24, 2018

View reviewed changes

pedro-ricardo changed the title ~~Minor highlight hotfix~~ Minor hotfixes Apr 28, 2018

More specific scope name for Fortran modules

6fc6737

pedro-ricardo mentioned this pull request Apr 29, 2018

Nested loop doesn't align properly #57

Closed

krvajal requested changes Apr 29, 2018

View reviewed changes

krvajal force-pushed the hotfix/highlight branch 2 times, most recently from b08eede to 6fc6737 Compare April 29, 2018 10:39

krvajal merged commit 30bd4d3 into master Apr 29, 2018

pedro-ricardo deleted the hotfix/highlight branch April 29, 2018 11:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor hotfixes #53

Minor hotfixes #53

pedro-ricardo commented Apr 18, 2018

codecov bot commented Apr 18, 2018 •

edited

Loading

krvajal left a comment

krvajal Apr 18, 2018

pedro-ricardo Apr 24, 2018 •

edited

Loading

krvajal Apr 18, 2018

krvajal commented Apr 24, 2018 via email

pedro-ricardo commented Apr 24, 2018

pedro-ricardo commented Apr 24, 2018

krvajal commented Apr 26, 2018

krvajal commented Apr 26, 2018 •

edited

Loading

pedro-ricardo commented Apr 26, 2018 •

edited

Loading

pedro-ricardo commented Apr 26, 2018

krvajal commented Apr 26, 2018

krvajal commented Apr 26, 2018

pedro-ricardo commented Apr 28, 2018

krvajal commented Apr 28, 2018 via email

pedro-ricardo commented Apr 29, 2018

krvajal commented Apr 29, 2018

krvajal Apr 28, 2018

Minor hotfixes #53

Minor hotfixes #53

Conversation

pedro-ricardo commented Apr 18, 2018

codecov bot commented Apr 18, 2018 • edited Loading

Codecov Report

krvajal left a comment

Choose a reason for hiding this comment

krvajal Apr 18, 2018

Choose a reason for hiding this comment

pedro-ricardo Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

krvajal Apr 18, 2018

Choose a reason for hiding this comment

krvajal commented Apr 24, 2018 via email

pedro-ricardo commented Apr 24, 2018

pedro-ricardo commented Apr 24, 2018

krvajal commented Apr 26, 2018

krvajal commented Apr 26, 2018 • edited Loading

pedro-ricardo commented Apr 26, 2018 • edited Loading

pedro-ricardo commented Apr 26, 2018

krvajal commented Apr 26, 2018

krvajal commented Apr 26, 2018

pedro-ricardo commented Apr 28, 2018

krvajal commented Apr 28, 2018 via email

pedro-ricardo commented Apr 29, 2018

krvajal commented Apr 29, 2018

krvajal Apr 28, 2018

Choose a reason for hiding this comment

codecov bot commented Apr 18, 2018 •

edited

Loading

pedro-ricardo Apr 24, 2018 •

edited

Loading

krvajal commented Apr 26, 2018 •

edited

Loading

pedro-ricardo commented Apr 26, 2018 •

edited

Loading