--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
Just for the information: see also the "redmine_rouge":/plugins/redmine_rouge (https://github.com/ngyuki/redmine_rouge) plugin.
--------------------------------------------------------------------------------
Mischa The Evil wrote:
> Just for the information: see also the "redmine_rouge":/plugins/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.

!redmine_rouge_plugin_csharp.png!
--------------------------------------------------------------------------------
This is a patch to replace CodeRay with Rouge.

With this patch applied, we can highlight "100+ languages":https://github.com/jneen/rouge/wiki/List-of-supported-languages-and-lexers 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.

!{width: 425px;}.highlight-sample.png!
--------------------------------------------------------------------------------
I tested the patch and I've the following observations:
# We should rename the "codeRay" variable in source:trunk/public/javascripts/jstoolbar/jstoolbar.js#L378
# Rename the "CodeRay" from source:trunk/public/stylesheets/rtl.css#L375
# 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:
!without_opening_tag.png!

With open tag:
!with_open_tag.png!

I think is related to this "issue":https://github.com/jneen/rouge/issues/187. I tried the workaround from there and it doesn't work.
--------------------------------------------------------------------------------
Marius BALTEANU wrote:
> I tested the patch and I've the following observations:
> # We should rename the "codeRay" variable in source:trunk/public/javascripts/jstoolbar/jstoolbar.js#L378
> # Rename the "CodeRay" from source:trunk/public/stylesheets/rtl.css#L375
> # 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.

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

Before fix:
!multiline-comment-before.png!

After fix:
!multiline-comment-after.png!
--------------------------------------------------------------------------------
Rouge 2.0.7 supports 113 languages.
The following is the list of supported languages.

{{collapse(Show the list of supported languages, Hide)
Languages written in *bold* are supported by CodeRay.

| abap | SAP - Advanced Business Application Programming |
| actionscript | ActionScript [aliases: as,as3] |
| apache | configuration files for Apache web server |
| apiblueprint | Markdown based API description language. [aliases: apiblueprint,apib] |
| applescript | The AppleScript scripting language by Apple Inc. (http://developer.apple.com/applescript/) [aliases: applescript] |
| biml | BIML, Business Intelligence Markup Language |
| bsl | The 1C:Enterprise programming language |
| *c* | *The C programming language* |
| ceylon | Say more, more clearly. |
| cfscript | CFScript, the CFML scripting language [aliases: cfc] |
| *clojure* | *The Clojure programming language* (clojure.org) [aliases: clj,cljs] |
| cmake | The cross-platform, open-source build system |
| coffeescript | The Coffeescript programming language (coffeescript.org) [aliases: coffee,coffee-script] |
| common_lisp | The Common Lisp variant of Lisp (common-lisp.net) [aliases: cl,common-lisp,elisp,emacs-lisp] |
| conf | A generic lexer for configuration files [aliases: config,configuration] |
| coq | Coq (coq.inria.fr) |
| *cpp* | *The C++ programming language* [aliases: c++] |
| csharp | a multi-paradigm language targeting .NET [aliases: c#,cs] |
| *css* | *Cascading Style Sheets, used to style web pages* |
| d | The D programming language(dlang.org) [aliases: dlang] |
| dart | The Dart programming language (dartlang.com) |
| *diff* | *Lexes unified diffs or patches* [aliases: patch,udiff] |
| docker | Dockerfile syntax [aliases: dockerfile] |
| eiffel | Eiffel programming language |
| elixir | Elixir language (elixir-lang.org) [aliases: elixir,exs] |
| *erb* | *Embedded ruby template files* [aliases: eruby,rhtml] |
| erlang | The Erlang programming language (erlang.org) [aliases: erl] |
| factor | Factor, the practical stack language (factorcode.org) |
| fortran | Fortran 95 Programming Language |
| fsharp | F# (fsharp.net) |
| gherkin | A business-readable spec DSL ( github.com/cucumber/cucumber/wiki/Gherkin ) [aliases: cucumber,behat] |
| glsl | The GLSL shader language |
| *go* | *The Go programming language* (http://golang.org) [aliases: go,golang] |
| gradle | A powerful build system for the JVM |
| *groovy* | *The Groovy programming language* (http://www.groovy-lang.org/) |
| *haml* | *The Haml templating system for Ruby* (haml.info) [aliases: HAML] |
| handlebars | the Handlebars and Mustache templating languages [aliases: hbs,mustache] |
| haskell | The Haskell programming language (haskell.org) [aliases: hs] |
| *html* | *HTML, the markup language of the web* |
| http | http requests and responses |
| idlang | Interactive Data Language |
| ini | the INI configuration format |
| io | The IO programming language (http://iolanguage.com) |
| *java* | *The Java programming language* (java.com) |
| *javascript* | *JavaScript, the browser scripting language* [aliases: js] |
| jinja | Django/Jinja template engine (jinja.pocoo.org) [aliases: django] |
| *json* | *JavaScript Object Notation* (json.org) |
| json-doc | JavaScript Object Notation with extenstions for documentation |
| jsonnet | An elegant, formally-specified config language for JSON |
| jsx | jsx [aliases: jsx,react] |
| julia | The Julia programming language [aliases: jl] |
| kotlin | Kotlin <http://kotlinlang.org> |
| liquid | Liquid is a templating engine for Ruby (liquidmarkup.org) |
| literate_coffeescript | Literate coffeescript [aliases: litcoffee] |
| literate_haskell | Literate haskell [aliases: lithaskell,lhaskell,lhs] |
| llvm | The LLVM Compiler Infrastructure (http://llvm.org/) |
| *lua* | *Lua* (http://www.lua.org) |
| make | Makefile syntax [aliases: makefile,mf,gnumake,bsdmake] |
| markdown | Markdown, a light-weight markup language for authors [aliases: md,mkd] |
| matlab | Matlab [aliases: m] |
| moonscript | Moonscript (http://www.moonscript.org) [aliases: moon] |
| mxml | MXML |
| nasm | Netwide Assembler |
| nginx | configuration files for the nginx web server (nginx.org) |
| nim | The Nim programming language (http://nim-lang.org/) [aliases: nimrod] |
| objective_c | an extension of C commonly used to write Apple software [aliases: objc] |
| ocaml | Objective CAML (ocaml.org) |
| *pascal* | *a procedural programming language commonly used as a teaching language.* |
| perl | The Perl scripting language (perl.org) [aliases: pl] |
| *php* | *The PHP scripting language* (php.net) [aliases: php,php3,php4,php5] |
| *plaintext* | *A boring lexer that doesn't highlight anything* [aliases: text] |
| powershell | powershell [aliases: posh] |
| praat | The Praat scripting language (praat.org) |
| prolog | The Prolog programming language (http://en.wikipedia.org/wiki/Prolog) [aliases: prolog] |
| prometheus | prometheus [aliases: prometheus] |
| properties | .properties config files for Java |
| protobuf | Google's language-neutral, platform-neutral, extensible mechanism for serializing structured data [aliases: proto] |
| puppet | The Puppet configuration management language (puppetlabs.org) [aliases: pp] |
| *python* | *The Python programming language (python.org)* [aliases: py] |
| qml | QML, a UI markup language [aliases: qml] |
| r | The R statistics language (r-project.org) [aliases: r,R,s,S] |
| racket | Racket is a Lisp descended from Scheme (racket-lang.org) |
| *ruby* | *The Ruby programming language* (ruby-lang.org) [aliases: rb] |
| rust | The Rust programming language (rust-lang.org) [aliases: rs] |
| *sass* | *The Sass stylesheet language language* (sass-lang.com) |
| scala | The Scala programming language (scala-lang.org) [aliases: scala] |
| scheme | The Scheme variant of Lisp |
| scss | SCSS stylesheets (sass-lang.com) |
| sed | sed, the ultimate stream editor |
| shell | Various shell languages, including sh and bash [aliases: bash,zsh,ksh,sh] |
| shell_session | A generic lexer for shell session and command line [aliases: terminal,console] |
| slim | The Slim template language |
| smalltalk | The Smalltalk programming language [aliases: st,squeak] |
| smarty | Smarty Template Engine [aliases: smarty] |
| sml | Standard ML [aliases: ml] |
| *sql* | *Structured Query Language, for relational databases* |
| swift | Multi paradigm, compiled programming language developed by Apple for iOS and OS X development. (developer.apple.com/swift) |
| tap | Test Anything Protocol [aliases: tap] |
| tcl | The Tool Command Language (tcl.tk) |
| tex | The TeX typesetting system [aliases: TeX,LaTeX,latex] |
| toml | the TOML configuration format (https://github.com/mojombo/toml) |
| tulip | the tulip programming language (twitter.com/tuliplang) [aliases: tulip] |
| turtle | Terse RDF Triple Language, TriG |
| twig | Twig template engine (twig.sensiolabs.org) |
| typescript | TypeScript, a superset of JavaScript [aliases: ts] |
| vala | A programming language similar to csharp. |
| vb | Visual Basic [aliases: visualbasic] |
| verilog | The System Verilog hardware description language |
| vhdl | Very High Speed Integrated Circuit Hardware Description Language |
| viml | VimL, the scripting language for the Vim editor (vim.org) [aliases: vim,vimscript,ex] |
| vue | Vue.js single-file components [aliases: vuejs] |
| *xml* | *<desc for="this-lexer">XML</desc>* |
| *yaml* | *Yaml Ain't Markup Language* (yaml.org) [aliases: yml] |
}}
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
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:*

|_<.No.: |_<.Filename: |_<.Comments: |
|_\3.Ruby code from CodeRay Scanner Test repository |
|o01. |[def.in.rb] |Rouge breaks on the ampersands in front of the blocks. |
|o02. |[diffed.in.rb] |With Rouge no clear difference between regex and string highlighting, and operator keywords aren't highlighted. |
|o03. |[operators.in.rb] |Rouge doesn't recognize the aliases, gives the at signs the error class and doesn't highlight method definitions where it should. |
|o04. |[quotes.in.rb] |Rouge breaks on the complex quoted literals and doesn't differ between different parts of regexes. CodeRay seems to break on the ampersand within the regex. |
|o05. |[regexp.in.rb] |Rouge doesn't highlight code inside regexes, doesn't highlight escape sequences within regexes. |
|o06. |[ruby19.in.rb] |Rouge seems to have problems taking in the Ruby 1.9 hash syntaxes. |
|o07. |[ruby2.in.rb] |Rouge breaks on the ampersands, messes up the keyword argument symbol highlighting, highlights extend as pseudo-keyword, highlights self incorrect, doesn't differ (clearly) between the %i{} and %w{} literals (maybe even some other %* literals too), highlights Ruby 2.1 syntax wrong, doesn't highlight Ruby 2.2 hash literal symbol keys with colons/quotes correctly and has issues highlighting the Ruby 2.3 squiggly heredoc. CodeRay doesn't differ class names from module names in definitions, which Rouge does. |
|o08. |[strange.in.rb] |Rouge does not seem to differ floats from constants correctly, does not recognize/differ backticked shell code (using both ` and %x), does not differ between inline instance-/class-/global variables, doesn't differ modifiers within regex constructs, misses several distinctions (char vs content, delimiter vs constant), has troubles with nested code which causes the rendering of the highlighting of the rest of the code completely broken. |
|o09. |[undef.in.rb] |Rouge breaks on the / method. Both Rouge and CodeRay break on the ampersands. |
|o10. |[unicode.in.rb] |Rouge does not seem to differ between integers and floats, and it renders unicode chars with class error — which seems wrong. |
|_\3.Diff 'code' from CodeRay Scanner Test repository |
|o11. |[diff.in.diff] |Rouge differs in what it highlights in comparison with CodeRay, where Rouge seems to put the attention more on the meta-data of the patch (diff command, indexes) and CodeRay more to the changes itself (filename, linenumbers). Rouge doesn't support inline change highlighting at all. |
|o12. |[github.in.diff] |See above. No inline change highlight. |
|o13. |[heredoc.in.diff] |CodeRay seems to do some inline highlighting of heredocs which Rouge doesn't (this seems to be a curious thing as far as I understand it fully). |
|_\3.HTML code from CodeRay Scanner Test repository |
|o14. |[cdata.in.html] |Rouge does not mark the inside of the cdata blocks in the inline javascript and css comments and renders the < as a unicode char with class error. |
|o15. |[redmine.in.html] |Rouge has the same cdata block issue as above. Rouge doesn't highlight inline css and javascript, and does not highlight html entities. Redmine breaks both highlighters due to incorrect handling of code tags. |
|_\3.Ruby code from redmine source and issue |
|o16. |[application_helper.rb] |Rouge incorrectly highlights attr as pseudo keyword. CodeRay has more distinct highlighting of escape sequences within regexes in contrast to Rouge. Both CodeRay and Rouge break on the ampersands (Rouge multiple times, CodeRay once). CodeRay provides more distinct string highlighting. CodeRay provides better distinction of regexes vs strings than Rouge. CodeRay does not highlight buildin methods (name, const_defined?, lambda, class_eval, etc.) specially as like Rouge does (as they are just methods which CodeRay doesn't highlight in any way). |
|o17. |[multiline_comment.rb] |Rouge omits highlighting of the she-bang line. |
|o18. |[project_nested_set.rb] |Here it becomes clear IMO that Rouge's method highlighting can be a real good thing (see eg. the calls to the *_changed? methods within the lambda and the method chains of self.class.where...where...maximum and self.class.where...pluck...first, etc). |

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":https://github.com/jneen/rouge/blob/master/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:
> > # We should rename the "codeRay" variable in source:trunk/public/javascripts/jstoolbar/jstoolbar.js#L378
> > # 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:*

Total files: 4 (22 unpacked):
* 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.

--------------------------------------------------------------------------------
A happy new year, Mischa. I am very impressed and deeply grateful for your deep inspection for Rouge and the patches.

As you wrote, I have to admit that there are some problems for 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).

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.

> 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?

I just replaced the variable name with an editor.

> 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.

Thanks, I will fix it.

--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------
Go MAEDA wrote:
> Mischa, could you review the new patch on #24681#note-16?

I will, and I'll also try yet another approach building upon yours.
I have a quick question in advance though (which holds true for both patch approaches): unpatched @highlight_by_filename@ seem to render unknown content (@language@ == false) using @ERB::Util.h@ and this behaviour seems to change in both your patches while @highlight_by_language@ seems to gain this behaviour by setting @lexer@ to @::Rouge::Lexers::PlainText@ when the content isn't recognized. Is this correct and wanted? Can you elaborate on that?
--------------------------------------------------------------------------------
Mischa The Evil wrote:
> I have a quick question in advance though (which holds true for both patch approaches): unpatched @highlight_by_filename@ seem to render unknown content (@language@ == false) using @ERB::Util.h@ and this behaviour seems to change in both your patches while @highlight_by_language@ seems to gain this behaviour by setting @lexer@ to @::Rouge::Lexers::PlainText@ when the content isn't recognized. Is this correct and wanted? Can you elaborate on that?

I wrote the code to avoid "RuntimeError: unknown lexer" exception. But I looked the original syntax_highlight.rb again and I realized that the exception will be catched in Redmine::SyntaxHighlighting.highlight_by_language and then @ERB::Util.h(text)@ will be performed. So, I don't have to set @lexer@ to @::Rouge::Lexers::PlainText@.

Thanks for pointing it out.
--------------------------------------------------------------------------------
Mischa The Evil wrote:
> I have a quick question in advance though (which holds true for both patch approaches): unpatched @highlight_by_filename@ seem to render unknown content (@language@ == false) using @ERB::Util.h@ and this behaviour seems to change in both your patches while @highlight_by_language@ seems to gain this behaviour by setting @lexer@ to @::Rouge::Lexers::PlainText@ when the content isn't recognized. Is this correct and wanted? Can you elaborate on that?

I think it is no problem to use @::Rouge::Lexers::PlainText@ as a lexer because it does nothing and finally @Rouge.highlight(text, ::Rouge::Lexers::PlainText, ::Rouge::Formatters::HTML)@ returns almost the same HTML with @ERB::Util.h@.

And patched version of @highlight_by_filename@ also uses @::Rouge::Lexers::PlainText@ for unknown languages because @::Rouge::Lexer.guess_by_filename@ returns @::Rouge::Lexers::PlainText@ if Rouge cannot guess language.
--------------------------------------------------------------------------------
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?
--------------------------------------------------------------------------------
Go MAEDA wrote:
> The patch attachment:0001-Syntax-highlighter-fall-back-to-Rouge-if-the-languag.patch is ready to merge. [...] Let's include this feature in 3.4.0.

As I've said in note-18, I have been working on extensively reviewing this. My preliminary conclusion is that the here mentioned patch should not yet be committed as its approach has some (major) drawback(s) and the implementation as-is comes with a big performance drawback. I'll wrap-up my review (including some clear data supporting my previous performance statement) and create/wrap-up an alternative patch this weekend.

--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------
Jean-Philippe, FYI: the conclusion of the discussion was that CodeRay's syntax highlighting seems to be significantly better quality for all 3 tested languages, but its list of supported languages is minuscule. That is why the combined approach was implemented in the final patch.
--------------------------------------------------------------------------------
This might be inappropriate, but have you considered re-using what GitHub uses ?

I'd say they should have a rather mature solution to support lots of language while keeping a good quality. It seems to live there: https://github.com/github/linguist
--------------------------------------------------------------------------------
For what it's worth, I think the idea of using CodeRay with a Rouge fallback is awesome. If you can make it work (the HTML output and formatting is pretty much incompatible), it would be the best of both worlds. I would also propose that for some languages, Rouge support is better (eg. Java and PHP).

As the maintainer of CodeRay, I can assure you that it will never support 100+ languages - at least not in its current form (version 2.0 is vaporware so far). Rouge is actively being developed, and support for JavaScript ES6, Swift, or other nice modern languages will probably never come to CodeRay.

@Mischa: Wow, awesome analysis! It may be a bit unfair to use CodeRay's test suite to compare Rouge, but the quality vs. quantity aspect is the whole reason CodeRay exists. I think the ampersand issues are related to the Redmine integration, though.

@Adrien: Linguist is great, but it requires Python as far as I'm aware…which would make it a bad fit for Redmine, right?

@Jean-Philippe: If there would be a "coderouge" meta-gem, combining coderay and rouge, would you accept it as "1 code highlighter"? I can reach out to jneen, the maintainer of Rouge, and see if we can release such a thing. Might even be useful for other projects.
--------------------------------------------------------------------------------
I am reading this with interest as I use a number of languages Coderay doesn't serve.

Is there a separate ready-made plugin available for rouge or does it require the patch in question?
--------------------------------------------------------------------------------
Yes, redmine needs better syntax highlighting - my Swift and Objective-C code does not show any highlighting... So I like the approach of having the existing Coderay but also a fallback to Rouge. It is better than having nothing at all...
--------------------------------------------------------------------------------
GitLab CE uses Rouge as syntax highlighter (we can find @gem 'rouge'@ in https://github.com/gitlabhq/gitlabhq/blob/master/Gemfile). It works well and most people are satisfied with it, I think.

Although Rouge may be imperfect as Mischa pointed out in #24681#note-14, I think supporting 100+ language is valuable for most Redmine users.
--------------------------------------------------------------------------------
I agree that supporting more languages is valuable. We have to keep in mind that this is only used to help reading code, not actual coding. So even if there are imperfection I believe most of them would be overlooked anyway.

I am favor of implementing only rouge, as it is most likely simpler to do and thus could be available sooner.
--------------------------------------------------------------------------------
+1 for moving to Rouge only.
--------------------------------------------------------------------------------
For anyone that wants to take advantage of the great work Go has done here but doesn't want to patch their installation I've packaged this up as a plugin:

https://github.com/alexbevi/redmine_rouge_highlighter
--------------------------------------------------------------------------------
Alex, thank you for the plugin, I just installed it, and it seems to work great for browsing repository content. However it doesn't seems to do anything for user typed content (issues, wiki, etc.). Is this the expected behavior ?

I suppose Go Maeda could also answer that question, since he wrote the patch in the first place.

In case it might be useful, here's my env:

<pre>
Environment:
Redmine version 3.3.3.stable
Ruby version 2.2.6-p396 (2016-11-15) [x86_64-linux-gnu]
Rails version 4.2.7.1
Environment production
Database adapter Mysql2
SCM:
Subversion 1.9.3
Git 2.7.4
Filesystem
Xitolite 2.7.4
Redmine plugins:
redmine_bootstrap_kit 0.2.5
redmine_git_hosting 1.2.2
redmine_github_hook 2.2.0
redmine_image_clipboard_paste 3.3.0
redmine_rouge_highlighter 0.0.1
redmine_tags 3.2.0
</pre>
--------------------------------------------------------------------------------
Adrien, I didn't actually test with anything other than the repository browser. I can have a look in a bit though as I'm not sure how they're different off the top of my head
--------------------------------------------------------------------------------
Adrien, I've addressed this issue in the plugin by changing the check for supported lexers from using CodeRay's list to Rouge's list. This still attempts to use CodeRay first then properly falls back to Rouge (see https://github.com/alexbevi/redmine_rouge_highlighter/commit/f9ae06978e48c15c2e13ae726cf311c1af20fe1c)
--------------------------------------------------------------------------------
This is great, it works well with Markdown instead of Textile too. I finally got sh syntax highlighting among many other things. Thank you !
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
A lot of users (12 in this thread) expressed they would rather have many ok-quality languages, rather than a few high-quality languages. Even Jean-Philippe seems to "agree to move to Rouge":https://www.redmine.org/issues/24681#note-23. So far only Mischa spoke against it. The last time he spoke was "over 1 year ago":https://www.redmine.org/issues/24681#note-22 saying he would come up with more data. But it seems he never got to it.

Can't we move on and replace Coderay with Rouge once and for all ?

Go MAEDA, would you be able to merge this for 4.0.0 ?
--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
+1 :)
--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
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":https://www.benfrederickson.com/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.

|_. Rank|_. Language |_. CodeRay |_. Rouge |
| 1 | JavaScript |=. x |=. x |
| 2 | Python |=. x |=. x |
| 3 | Java |=. x |=. x |
| 4 | C++ |=. x |=. x |
| 5 | C |=. x |=. x |
| 6 | PHP |=. x |=. x |
| 7 | C# |=. |=. x |
| 8 | Shell |=. |=. x |
| 9 | Go |=. x |=. x |
| 10 | TypeScript |=. |=. x |
| 11 | Ruby |=. x |=. x |
| 12 | Jupyter Notebook |=. |=. |
| 13 | Objective-C |=. |=. x |
| 14 | Swift |=. |=. x |
| 15 | Kotlin |=. |=. x |
| 16 | R |=. |=. x |
| 17 | Scala |=. |=. x |
| 18 | Rust |=. |=. x |
| 19 | Lua |=. x |=. x |
| 20 | Matlab |=. |=. x |
| 21 | PowerShell |=. |=. x |
| 22 | CoffeeScript |=. |=. x |
| 23 | Perl |=. |=. x |
| 24 | Groovy |=. x |=. x |
| 25 | Haskell |=. |=. x |

--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> 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 removed CodeRay in the latest patch.

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
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:

p=. !ruby_broken.png!

--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> 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:

Thank you for reviewing, I will check it in a hurry.
--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> There's a serious problem with & caracters:

This problem occurs when the text formatting setting is Textile. redcloth3.rb replaces all "&" to "&#120;&#37;&#120;&#37;" before calling @highlight_by_language@ method (see #29681).

Maybe redcloth3.rb must replace "&#120;&#37;&#120;&#37;" with "&" before calling highlighter.
--------------------------------------------------------------------------------

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

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

To convert "&#120;&#37;&#120;&#37;" 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.
--------------------------------------------------------------------------------
Thank you Go MAEDA for not giving up this feature and everyone else who contributed to the patch being merged.
--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
This broke Java syntax again. :(
https://github.com/rouge-ruby/rouge/pull/1414

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------

relates,Closed,2623,C# syntax highlighting
relates,Closed,3032,Use google Prettify for syntax highlighting instead of CodeRay
relates,Closed,1313,Optionally use ultraviolet for syntax highlighting
relates,Closed,1651,Hack to make redmine use pygmentize instead of CodeRay
relates,Closed,28094,Kotlin code highlight support
relates,Closed,29259,Attachment preview does not work for some source files such as JavaScript and Go
relates,New,29681,"x%x%" is rendered as "&" in Textile formatter
relates,Closed,26708,Diff formatting results empty lines if they contains HTML tags
relates,Closed,20758,Ampersand+keyword in code highlighting
relates,Closed,30434,Line height is too large when previewing files with syntax highlighting if the line terminators are CRLF
relates,New,35676,Optimize performance of syntax highlighting implementation