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

issue in highlighting fortran code: kind of intrinsic data type #1723

Closed
kookma opened this issue Apr 6, 2018 · 24 comments · Fixed by #2379
Closed

issue in highlighting fortran code: kind of intrinsic data type #1723

kookma opened this issue Apr 6, 2018 · 24 comments · Fixed by #2379
Labels
enhancement An enhancement or new feature good first issue Should be easier for first time contributors help welcome Could use help from community

Comments

@kookma
Copy link

kookma commented Apr 6, 2018

In modern fortran you can set the kind of intrinsic data type. It means you can define additional intrinsic data type. When you do so, for literal constants you have to use the kind name attached to literal consonant using under score.

Example

PROGRAM test_initial
INTEGER, PARAMETER :: DBL = SELECTED_REAL_KIND(p=13)
REAL(KIND=DBL) :: a1 = 6.666666666666666
REAL(KIND=DBL) :: a2 = 6.666666666666666_DBL
WRITE (*,*) a1, a2
END PROGRAM test_initial

Highlight.js issue

When using highlight.js to highlight a Fortran code contains such named constant, highlight.js fails to do the job! It highlights the remaining code (from the position of named constant) in a wrong way!

Note

  • The above code in this post is highlighted by github, it works correctly! See the name constant is in blue color.
  • other intrinsic data can have other kind. But in all cases in named constants, the kind is specified by an underscore to the constant (e.g 12_ps)
@kookma
Copy link
Author

kookma commented Apr 20, 2018

Solution

This is the solution to the above question.
The "number" class in fortran.js file shall be corrected as bellow:

{
        className: 'number',
        begin: '(?=\\b|\\+|\\-|\\.)[+-]?(?:\\.|\\d+\\.?)\\d*([de][+-]?\\d+)?(_[a-z\\d]+)?\\b',
        relevance: 0
      }

@kookma
Copy link
Author

kookma commented Dec 24, 2018

Is there any chance to be merged in the new revision of main release?

@egor-rogov
Copy link
Collaborator

@kookma Could you make a PR out of this (plus some tests)?

@kookma
Copy link
Author

kookma commented Dec 24, 2018

Sure, but I don't know how to make a PR? Should I fork this repo?

@egor-rogov
Copy link
Collaborator

Yes, first fork the repo and commit changes to it. Then in your repo on "Pull requests" tab find "New pull request" button.

@kookma
Copy link
Author

kookma commented Dec 25, 2018

@egor-rogov
Sure, I do it.

@kookma
Copy link
Author

kookma commented Dec 25, 2018

@egor-rogov
Please have a look at: #1934

@joshgoebel joshgoebel added bug help welcome Could use help from community good first issue Should be easier for first time contributors labels Oct 7, 2019
@joshgoebel joshgoebel added this to the 9.15.12 milestone Oct 24, 2019
@joshgoebel
Copy link
Member

See also #1566

@kookma
Copy link
Author

kookma commented Oct 24, 2019

@yyyc514
Thank you for following up this issue!
Highlight.js is used in many webpages have Fortran stuffs so it worth to correct the issue!
I saw #1566 and will try that!
My simple solution above works for me but I am not a regex expert to see if the solution is correct or not!

@kookma
Copy link
Author

kookma commented Oct 24, 2019

@yyyc514
I check the new fix on https://jsfiddle.net/ajoshguy/nagkqytv/11/
by trying the below code:

<pre>
<code class="fortran">
program main
! This is a test
real :: x, y
x=1.23_wp
y=0._wer
z=1.25e9
end program main
</code>
</pre>

And it seems you solution from #1566 works!

So, I think you can to close this issue.

@joshgoebel
Copy link
Member

Well, our desire is the highlight the WHOLE number, including the "kind", that's why this is still open. What should be fixed now if us "losing" the first digit, which was not good at all.

@kookma
Copy link
Author

kookma commented Oct 24, 2019

@yyyc514

What I checked at https://jsfiddle.net/ajoshguy/nagkqytv/11/ is


program main
! This is a test
real :: x, y
x=1.23_wp
y=0._wer
z=1.25e9
end program main

and it shows the whole number including the first number and the kind highlighted!
I mean number like 12.34_wp

I am afraid I don't understand what you explained.

@joshgoebel
Copy link
Member

Screen Shot 2019-10-24 at 3 37 15 PM

Not sure what you are seeing, but this is not correct yet. And it can't be since the regex hasn't been updated. Its' possible you're seeing some other language highlighting the code instead.

@kookma
Copy link
Author

kookma commented Oct 24, 2019

I see it as below, so it works at https://jsfiddle.net/ajoshguy/nagkqytv/11/
hilight-fortran

@joshgoebel
Copy link
Member

Not Fortran. :-) Did you tell it to load fortran in the init config?

@kookma
Copy link
Author

kookma commented Oct 24, 2019

Where is that? I just used <code class="fortran">
I assume it loads all language brushes!

@joshgoebel
Copy link
Member

joshgoebel commented Oct 24, 2019

No it loads only the common set. It's at the top of the file. It's meant to be as close as possible to an actual usage, so it would be a poor testing environment if it loaded EVERYTHING.

@kookma
Copy link
Author

kookma commented Oct 24, 2019

@yyyc514 okay, I changed the first like to var language = "fortran"; and now I see what you posted!

In my revised version as I explained above I use

{
        className: 'number',
        begin: '(?=\\b|\\+|\\-|\\.)[+-]?(?:\\.|\\d+\\.?)\\d*([de][+-]?\\d+)?(_[a-z\\d]+)?\\b',
        relevance: 0
      }

and it works fine. Have a look at

https://kookma.github.io/TW-Scripts/#Syntax%20Highlighting:%5B%5BSyntax%20Highlighting%5D%5D%20%24%3A%2Fplugins%2Ftiddlywiki%2Fhighlight%20%24%3A%2Fplugins%2Ftiddlywiki%2Fhighlight%2Fhighlight.js

Here I use highlight.js as plugin! Scroll down to see the modified highlight.js

@kookma
Copy link
Author

kookma commented Oct 24, 2019

You can edit online the content and check Fortran code of your own!
Edit change and then push the save button!

@joshgoebel
Copy link
Member

joshgoebel commented Oct 24, 2019

Yeah, we've been debating on the correct/best match rule. That might wait till .12 though, depends how much time I have between now and release.

My priority was first fixing the data loss.

@kookma
Copy link
Author

kookma commented Oct 24, 2019

Okay! Thank you for all your efforts.

@joshgoebel joshgoebel added this to .12 Plan in Highlight.js Oct 25, 2019
@joshgoebel joshgoebel moved this from .12 Plan to Contributors in Highlight.js Oct 25, 2019
@joshgoebel joshgoebel added enhancement An enhancement or new feature and removed bug labels Oct 25, 2019
@joshgoebel joshgoebel moved this from Backlog to Next Up in Highlight.js Dec 7, 2019
@joshgoebel joshgoebel modified the milestones: 9.17, 9.18 Dec 7, 2019
@joshgoebel joshgoebel modified the milestones: 9.18, 10.0 Dec 23, 2019
joshgoebel added a commit to joshgoebel/highlight.js that referenced this issue Jan 31, 2020
joshgoebel added a commit to joshgoebel/highlight.js that referenced this issue Jan 31, 2020
Highlight.js automation moved this from Next Up to Done Feb 6, 2020
joshgoebel added a commit that referenced this issue Feb 6, 2020
…r detection (#2379)

* enh(fortran) support intrinsic data types

Closes #1723.

* (parser) throw "0 width match" error for bad regex

Closes #2140.

- In safe mode 0 width matches will be safely and quietly ignored
  and advance the cursor 1 step, as before.
- In debug mode a "0 width match" error will be thrown.

This should help prevent such misbehaved rules from sneaking back
into the library in the future.
taufik-nurrohman pushed a commit to taufik-nurrohman/highlight.js that referenced this issue Feb 18, 2020
…r detection (highlightjs#2379)

* enh(fortran) support intrinsic data types

Closes highlightjs#1723.

* (parser) throw "0 width match" error for bad regex

Closes highlightjs#2140.

- In safe mode 0 width matches will be safely and quietly ignored
  and advance the cursor 1 step, as before.
- In debug mode a "0 width match" error will be thrown.

This should help prevent such misbehaved rules from sneaking back
into the library in the future.
@kookma
Copy link
Author

kookma commented Mar 14, 2020

@yyyc514
I have checked 9.18.1 the issue #1723 (comment)

still there!

@joshgoebel
Copy link
Member

The full fix should be in 10.0. Pretty sure we closed this one down. Please test on the 10.0 beta.

@kookma
Copy link
Author

kookma commented Mar 14, 2020

many thanks Josh!

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
Projects
No open projects
Highlight.js
  
Done
3 participants