Skip to content

OpenACC highlighting #184

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

Closed
IncubatorShokuhou opened this issue Jun 12, 2020 · 15 comments · Fixed by #224
Closed

OpenACC highlighting #184

IncubatorShokuhou opened this issue Jun 12, 2020 · 15 comments · Fixed by #224

Comments

@IncubatorShokuhou
Copy link

IncubatorShokuhou commented Jun 12, 2020

I found that OpenMP codes beginning with !$ can be correctly highlighted. Is it possible to highlight OpenACC codes, just like what has been done on OMP codes?

@gnikit
Copy link
Member

gnikit commented Apr 12, 2021

We use json files for our syntax highlighting do you have any resources as to where could we find something similar for OpenACC? If not we can always write one, but given that I don't personally use OpenACC help will probably be needed.

@IncubatorShokuhou
Copy link
Author

This is the official site of OpenACC: https://www.openacc.org/
This is the API Guide of OpenACC(which I think might be the most useful resource for you): https://www.openacc.org/sites/default/files/inline-files/API%20Guide%202.7.pdf
Hopefully these will help.

@gnikit
Copy link
Member

gnikit commented Apr 14, 2021

Thanks for the link, I had a look and from what I can tell it needs a bit of work to implement since we will have to write the regex json file ourselves. I do think however (from quickly looking over the syntax of both) that there is substantial overlap between OpenACC and OpenMP syntax so we might be able to get away with some quick and dirty copy-pasting as a first attempt.

@jefflarkin
Copy link

Please see #224.

@jefflarkin
Copy link

Thanks for the link, I had a look and from what I can tell it needs a bit of work to implement since we will have to write the regex json file ourselves. I do think however (from quickly looking over the syntax of both) that there is substantial overlap between OpenACC and OpenMP syntax so we might be able to get away with some quick and dirty copy-pasting as a first attempt.

Kinda funny, I happened to already be working on this before I saw your comment. Indeed there's a significant amount of overlap with OpenMP and I used its syntax as the starting point. It appears to me that anything that includes #variable-list is not matching properly in OpenMP, so I removed it from the OpenACC until it's fixed. I also had to make the matching for !$omp slightly less greedy because it would sometimes match things within the OpenACC directive.

@gnikit
Copy link
Member

gnikit commented Apr 17, 2021

It appears to me that anything that includes #variable-list is not matching properly in OpenMP, so I removed it from the OpenACC until it's fixed.

I would suggest you open an issue for this, how it should be vs how it currently is and we can tackle it ASAP.

I also had to make the matching for !$omp slightly less greedy because it would sometimes match things within the OpenACC directive.

About this, I would be very cautious when doing changes in the highlighting bit of the extension since it is not tested in any way. With that being said, is the change expected to cause OpenMP to fail at highlighting certain pieces of code.

In general I think that the highlighting for OpenXXX stuff should be using the same base where possible, like the preprocessor, so it might be worth loading bits of OpenMP to the OpenACC instead of copy-pasting the file.

@gnikit gnikit linked a pull request Jun 30, 2021 that will close this issue
@jefflarkin
Copy link

@gnikit Is there an eta for when a new build will go into the market that includes this support?

@gnikit
Copy link
Member

gnikit commented Jul 23, 2021

@krvajal could we issue a new version from the existing master? We can deal with PRs #238 and #244 in the next release.

@jefflarkin We need to figure out a way for us to publish to the Microsoft store from within github without having to call on krvajal every time.

@jefflarkin
Copy link

I see in the marketplace that the extension has updated to 2.3.0 and lists OpenACC support, but it doesn't appear to be firing the syntax highlighting on any of my computers. I also notice that there's no 2.3.0 release shown on GitHub. Was the extension actually updated? Thanks.

@gnikit
Copy link
Member

gnikit commented Sep 15, 2021

Hi @jefflarkin I believe it has. This is what I get when I open the test file
Screenshot from 2021-09-15 15-20-13

@jefflarkin
Copy link

Yeah, that's not right, it's only highlighting keywords that are common with OpenMP. When I submitted the pull request nearly every green word in that screenshot was highlighted red. I'm trying to rebuild on my machine to create a screenshot of what I was getting at that time.

@gnikit
Copy link
Member

gnikit commented Sep 15, 2021

You don't have to rebuild, if the highlighting is not getting picked up it has to do with the syntaxes folder which you can edit on the fly. Let me have a look as well.

@gnikit gnikit reopened this Sep 15, 2021
@gnikit
Copy link
Member

gnikit commented Sep 15, 2021

Okay so good news and bad news. I just had a quick look and it appears that the openacc scope is not the one being used, but rather the extension uses the openmp one, bad news is I have no idea how to fix it yet. This looks like it should have a relatively easy resolution. I will have a look later tonight.

@gnikit
Copy link
Member

gnikit commented Sep 15, 2021

I found the bug, the regex for OpenMP was too aggressive. I will open a PR shortly and see this is merged into another release but in the meantime you might want to go change line 32 in openmp_lang.json to "begin": "(?i)\\G(\\$(omp\\b)&?)",

@gnikit gnikit closed this as completed Sep 15, 2021
@jefflarkin
Copy link

Darn! I thought we'd fixed the over-greedy OpenMP regexp, but I guess this slipped past. Thanks for debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants