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

Implement R code highlight #5102

Merged
merged 5 commits into from
Jun 13, 2023
Merged

Conversation

shun2wang
Copy link
Contributor

@shun2wang shun2wang commented May 30, 2023

Close: https://github.com/jasp-stats/INTERNAL-jasp/issues/2328

Now looks like:
image

TODO:

  • Highlighting in exported HTML (Not now)
  • Highlighting in Help Docs
  • Style improvement
  • Performance Testing (EDIT: about 16ms for a r code syntax of one analysis)

@EJWagenmakers
Copy link
Contributor

awesome

Copy link
Contributor

@boutinb boutinb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice!!!
Just one remark.

Desktop/html/index-jasp.html Outdated Show resolved Hide resolved
@JorisGoosen
Copy link
Contributor

JorisGoosen commented May 30, 2023

Could we also use this to highlight the rcode in textareas in the gui?

Ive done some reading in the https://doc.qt.io/qt-6/qsyntaxhighlighter.html and some related pages.
It seems that we cant reuse the js highlighter.

But if we have a regex that can highlight R we could use that.

And looking in the PR I guess the highlighter itself already has some definition inside. Ill have a look at their docs

Edit: https://github.com/highlightjs/highlight.js/blob/main/src/languages/r.js seems quite useful for implementing this later.
But reading https://doc.qt.io/qt-6/qsyntaxhighlighter.html#highlightBlock indicates it might require a bunch more work.
Better postpone that.

Having R code look awesome in the results-notes will already be very nice.

@shun2wang
Copy link
Contributor Author

shun2wang commented May 30, 2023

Could we also use this to highlight the rcode in textareas in the gui?

Is there a way to import js directly in QML? However that seems to require using qmake build system? if so sounds terrible🤪

But if we have a regex that can highlight R we could use that.

I recommend here https://prismjs.com/download.html#themes=prism&languages=r

@JorisGoosen
Copy link
Contributor

Is there a way to import js directly in QML? However that seems to require using qmake build system? if so sounds terrible🤪

Well it is possible to include js directly for use in qml it wont then be available to the QSyntaxHighlighter without a bunch of C++ trickery. So it wouldnt make things much easier :p

prism.js

https://github.com/PrismJS/prism/blob/master/components/prism-r.js
It does look slightly cleaner

@shun2wang
Copy link
Contributor Author

shun2wang commented Jun 1, 2023

Now in help docs:

\`\`\`r
if (i == 0) {
  y = TRUE
} else {
  y = FALSE
}
\`\`\`

image

So I think it ready to review.
Legacy :
Implemented code highlighting for exporting HTML, in fact only needed to come with css styles, but I don't want to "add more css styles inline" right now so it's left to next time I clean up all styles. some of them in #5073

@shun2wang shun2wang marked this pull request as ready for review June 1, 2023 11:35
@shun2wang shun2wang requested review from boutinb and JorisGoosen June 1, 2023 11:35
Copy link
Contributor

@JorisGoosen JorisGoosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look fine and it works splendidly!

But...Dark theme support is still lacking for the rsyntax box:
image

Resources/Help/marked.js Show resolved Hide resolved
@JorisGoosen JorisGoosen self-requested a review June 1, 2023 16:09
@JorisGoosen
Copy link
Contributor

Looks good to me ^^

@JorisGoosen
Copy link
Contributor

@boutinb you think you can also review this?

@boutinb boutinb merged commit 999863b into jasp-stats:stable Jun 13, 2023
@shun2wang shun2wang deleted the rSyntaxHighlight branch June 13, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants