Redmine

Mischa The Evil wrote:

Just for the information: see also the redmine_rouge (https://github.com/ngyuki/redmine_rouge) plugin.

Thanks for the information. The plugin works fine on the current trunk (r16111).
It seems that we can implement this feature in a small amount of code.

This is a patch to replace CodeRay with Rouge.

With this patch applied, we can highlight 100+ languages including C# (csharp), Visual Basic (vb), Objective-C (objective_c), Swift (swift) and Perl (perl).

Users can use all language classes (code class="XXX") currently supported by CodeRay except for taskpaper.

Since the supported language dramatically increases from 24 to 100+, I think that the merit of this patch for users is very large.

1. We should rename the "codeRay" variable in source:trunk/public/javascripts/jstoolbar/jstoolbar.js#L378
2. Rename the "CodeRay" from source:trunk/public/stylesheets/rtl.css#L375
3. Update help documentation: source:trunk/public/help/en/wiki_syntax_detailed_markdown.html#L300

Also, the Rogue library seems to have an issue with the PHP language which is highlighted only when the opening tag is present.

Without open tag:

With open tag:

I think is related to this issue. I tried the workaround from there and it doesn't work.

Marius BALTEANU wrote:

I tested the patch and I've the following observations:

1. We should rename the "codeRay" variable in source:trunk/public/javascripts/jstoolbar/jstoolbar.js#L378
2. Rename the "CodeRay" from source:trunk/public/stylesheets/rtl.css#L375
3. Update help documentation: source:trunk/public/help/en/wiki_syntax_detailed_markdown.html#L300

Thanks for your feedback, I have fixed all of the above.

Also, the Rogue library seems to have an issue with the PHP language which is highlighted only when the opening tag is present.

Also fixed.

Fixed multiline comments highlighting issue in file view, the same problem that was reported as #7495 for CodeRay.

Before fix:

After fix:

Rouge 2.0.7 supports 113 languages.
The following is the list of supported languages.

Show the list of supported languagesHide

I'm currently wrapping-up a review of the proposed change and the corresponding patches provided by Go.

Done! Here it is...

I have spent some time testing the Rouge syntax highlighter using the patches provided by Go in contrast to the implementation provided by the redmine_rouge plugin (which seems to have an issue with the CSS-styles — thus the highlighting — missing during printing).
I specifically looked at the Ruby code syntax highlighting, but I also included two others: Diff and HTML (with inline CSS and Javascript).

Procedure:

I started with collecting some — let's say — more challenging code examples. I found some interesting ones (specifically for comparison of CodeRay vs. Rouge) in the CodeRay Scanner Test repository (https://github.com/rubychan/coderay-scanner-tests). I also took at random some long, more complicated methods from the Redmine core to compare.

With these examples in place, I added all the code to a single wikipage and dropped the files into the projects repository for testing of the repo view file function (my findings for the single wikipage are not different from the projects repository view file functionality, so I won't go into this specifically). I then printed the whole wikipage to a PDF (using Chrome's internal print function — this is actually what's not working using the redmine_rouge plugin) for easy and consistent comparison.

I did these tests (by means of anything better) on a fresh deployment of a Bitnami Redmine 3.3.1 stack. First I printed the wikipage as said above using the default CodeRay highlighter. Then I applied the patches (cherry-picked patches 0001, 0005 and 0006; omitting 0002, 0003 and 0004 patches) manually, restarted the whole stack and printed the same wikipage using the Rouge highlighter. Good to note is that I thus have tested Rouge's capabilities using the currently patched-in colorful style theme.

I think that the results I got (two, 9+ MB, 25-page, A3-landscape pdf's) are actually pretty clear albeit large — sneakpeak: I'm not very excited about them. I don't talk about performance here (I haven't tested that), but rather about the quality of the provided code highlighting of, particularly, the Ruby and the Diff code by the Rouge library. Along with this, I also see some issues with the current CodeRay highlighter (whether these are issues in the CodeRay library or the Redmine implementation, I don't know at the moment). I'll below elaborate on my observations during the review, using numbered items and referring to the filenames of the testfiles I have used. I have written it such that one can read/scan along the test code in the pdf's (preferably splitscreen) in roughly the same order.

Observations:

Two of the biggest differences that seem to be recurring along a multitude of the Ruby testfiles are due to differences in design perspective:

x1: Rouge highlights 'pseudo-keywords', CodeRay doesn't — for good reasons AFAICR.
x2: Rouge highlights methods (class 'nf'), CodeRay doesn't — again, for good reasons AFAICR.

Some conclusions:

A1: The tested patch series succesfully implements the replacement of CodeRay with Rouge (excluding conclusions B1 and B2, in case these are due to Redmine implementation(s)).
A2: The reported PHP issue and the additionally found multiline issue are succesfully fixed by the additional patches.
A3: We might be able to tweak the js-toolbar code button in some neat way to support (a selection of) popular languages added by Rouge.

B1: CodeRay (and Rouge) highlighting (within the current Redmine implementation) can break on code tags within (at least) HTML code.
B2: CodeRay (and [significantly more] Rouge) highlighting (within the current Redmine implementation) can break on ampersands within (at least) Ruby code.

C1: Rouge has a lot (more) issues highlighting Ruby/HTML/JS/CSS/Diff code correctly/sufficiently.
C2: Rouge misses some more essential Diff highlighting features.
C3: Rouge misses support for inline CSS/JS highlighting within HTML code.

D1: Overall I think that CodeRay performs better judging by the quality and features of the provided highlighting of the tested languages.
D2: Rouge does indeed provide support for way more languages than CodeRay does (and I really like that), but if the same kind of issues are among those languages too, I think we'd just be making a bad trade-of between quantity over quality if we switch away from CodeRay (at least at the moment).

As the observant reader may have already noticed above, I am a bit disappointed by the overall quality of the highlighting of the Rouge library in comparison to CodeRay. If you'd ask me whether or not to replace CodeRay with Rouge, I'd vote against such a switch now.
I do however think that a lot of people are willing to make this trade-of considering the amount of languages supported by Rouge. As such could it be a good idea to build upon the changes from #2985 and integrate additional support for the Rouge highlighter within the core while keeping CodeRay as the default (with a front-end UI in the form of a setting; just like done for the Redmine Text Formatting and as like was originally proposed by Jean-Baptiste in #2985). That shouldn't be all too difficult to get done and provides the users an easy way to make the choice for themselfs without having to rely on a third-party plugin.

Some additional comments:

Go MEADA wrote:

Citation from Rouge's README.md :

Advantages to CodeRay

• The HTML output from Rouge is fully compatible with stylesheets designed for pygments.
• The lexers are implemented with a dedicated DSL, rather than being hand-coded.
• Rouge supports every language CodeRay does and more.

Regarding the first: I don't see how this is an advantage of Rouge over CodeRay for the end user. Regarding the second: I think this may actually well be a drawback being (one of) the cause(s) of a some or more of the Ruby highlighting issues observed above in the testfiles. Regarding the third: well, I see taskpaper (which indeed isn't a language, true ;). Besides that one, I really think that with syntax highlighting it is a case of doing it good, then it's useful. Doing it wrong, then it's just pretty-coloring code and as such a waste of processor-cycles. In my opinion quality should go way over quantity on this matter.

Go MAEDA wrote:

Marius BALTEANU wrote:

I tested the patch and I've the following observations:

1. We should rename the "codeRay" variable in source:trunk/public/javascripts/jstoolbar/jstoolbar.js#L378
2. Rename the "CodeRay" from source:trunk/public/stylesheets/rtl.css#L375
   [...]

Thanks for your feedback, I have fixed all of the above.

Ragarding the first: you seem to know how the minified js file is created as you seem able to patch it. I — and I'm sure some others too — are quite interested in the used process. Do you want to elaborate on that? Regarding the second: as far as I know the linenumbers styles became unused after r10131 and were removed from regular stylesheets with r14487. As such is patch 0004 obsolete and should it be replaced by a complete removal of both the comment and the actual style definition from rtl.css.

Some additional comments about the in-/output files/code:

• One plain-text input document containing the content (of a wikipage) in textile: codehighlight.examples.txt;
• Two pdf output documents containing the rendered content from codehighlight.examples.txt as a wikipage for both highlighters: codehighlight.examples.coderay.pdf and codehighlight.examples.rouge.pdf;
• One zip-archive containing eighteen individual test code files: syntaxhl-testfiles.zip.

Because of the size limitations on redmine.org I have uploaded these four files to my old MediaFire-account (http://www.mediafire.com/evildev). I uploaded them to the subfolder 'rm24681' of the 'Redmine Miscellaneous' folder. A direct link to this shared folder is: https://www.mediafire.com/folder/9994htt2r7fdg/rm24681

Wrap-up:

If you have made it to here, yay. Seriously, apologies for the lenghty post! I thought let's start this new year by posting a nice detailed review... Best whishes for 2017 to all participants of this issue and beyond!

I'm interested in (further) feedback and willing to answer/participate in additional questions/discussions. Other experiences (eg. with other languages) from users using this patch or the plugin are very welcome if you ask me.

Kind regards, Mischa.

Go MAEDA wrote:

How about using both CodeRay and Rouge? It means that syntax highlighting will be basically processed by CodeRay and fall back to Rouge if the language is not supported by CodeRay. With this method, we can increase supported languages while avoiding deteriorating the quality of syntax highlighting. In addition, it is possible to prevent increase of admin settings.

I have created a new patch with a new approach: attachment:0001-Syntax-highlighter-fall-back-to-Rouge-if-the-languag.patch

• Syntax highlighting is mainly done by CodeRay.
• Rouge is used only when the given language is not supported by CodeRay.

We can increase supported language of syntax highlighting by this fallback mechanism while enjoying the high quality of CodeRay.

Mischa, could you review the new patch on #24681#note-16?

attachment:0001-Syntax-highlighter-fall-back-to-Rouge-if-the-languag.patch

I think it can make use of the good points of both CodeRay and Rouge.

The patch attachment:0001-Syntax-highlighter-fall-back-to-Rouge-if-the-languag.patch is ready to merge. I am sure that supporting over 100 languages brings great benefits to users. Let's include this feature in 3.4.0.

But I cannot write good English, so it is hard for me to update public/help/*/wiki_syntax_detailed_*.html. Could someone add explanation about Rouge to help files?

Thanks Mischa for reviewing this.

I didn't try the patches but it looks like CodeRay would still be used in some cases. Am I wrong? Because I'm not really in favor of keeping 2 code highlighters in the core.

I have updated the patch. Now it is compatible with the latest trunk (3.4.6.devel.17471).

I Still think that switching to Rouge is beneficial for most users. I understand Mischa's opinion that CodeRay deals with complex Ruby code very well. But I think they are edge cases. Rouge works fine against most codes. Rouge is already used by major projects such as Jekyll and Gitlab.

The biggest problem for me is that CodeRay does not support popular languages such as C#, Swift, and Kotlin. The following table shows that top 25 popular languages on GitHub (according to Ranking Programming Languages by GitHub Users) and support status by CodeRay and Rouge. Rouge supports 24 of 25 languages but CodeRay supports only 10 languages. To make matters worse, CodeRay does not support emerging languages such as TypeScript, Swift, and Kotlin.

I've applied the latest patch and gave it a try with the content of attachments_controller.rb. There's a serious problem with & caracters:

Fixed the ampersand issue. Please test 0001-Switch-syntax-highlighter-to-Rouge-from-CodeRay.patch.

The reason why ampersand is broken is that recloth3.rb replaces "&" with "x%x%" while processing Textile. if text "x = a & b" is given, it will be converted to "x = a x%x% b" temporarily and passed to syntax highlighter.

To convert "x%x%" back to "&" before processing with Rouge, I added gsub! method just before calling highlight_by_language in lib/redmine/wiki_formatting/textile/formatter.rb.

Thanks for fixing this issue, patch is committed.

r17532 comments out the gem 'puma', please see source:trunk/Gemfile#L92 and I don't think that it is a desired change.

I'm closing this because the unwanted change was reverted by r17537.