Redmine

I really, really like the proposed patch, especially for the HTML Pipeline gem that can help us a lot to improve the current code.

Temporary workaround before the patch is merged¶

At the moment, it is possible to use redmine_common_mark plugin. It also allows for configuring commonmarker, but it has no HTML sanitizing implemented. Actually I found this patch when I was about to offer a pull request with html-pipeline to the plugin's author.

We’d still appreciate merging this patch ASAP.

commonmarker configuration¶

These are the options used by GitLab. The options in the patch are the same (thumbs up!).

• STRIKETHROUGH_DOUBLE_TILDE is not used in repos.
• It uses HARDBREAKS in their issues, which is inconsistent with their wiki and repository rendering.
• They have the tasklist extension enabled. I'd consider enabling it in Redmine, as tasklits is an officially documented GFM feature.

I can imagine that some Redmine users would like other options than us, as we have seen before in Redmine. And it would be quite legit e.g. for the tasklists mentioned above.

Making commonmarker configurable can mean way too many config options, which is difficult to support. A good compromise might be to make a hook for plugins, so that they can change the config.

I can create a follow-up ticket after getting some feedback.

HTML sanitizing¶

HTML sanitizing is currently embedded in the commonmark formatter in the patch. That's a good start.

As #807 suggests, sanitizing should rather be a shared concept, eventually with small differences for different formats. GitLab did it this way, their base_sanitization_filter.rb contains common sanitization setup and CommonMark sanitization_filter.rb customizes the HTML::Pipeline::SanitizationFilter on top of that.

I can create a follow-up ticket after getting some feedback.

Textile to Markdown migration¶

Pandoc is actually bad at this job. We tried Jens'es redmine_convert_textile_to_markdown (thanks for that!), we ran rendering comparison tests¹ covering hundreds of thousands of strings and the results were poor.

¹ Rendering comparison test = grab all rendered issues and wiki pages from the Textile Redmine instance and a converted-to-Markdown Redmine instance, normalize HTML, compare the HTMLs.

So we forked it, and similarly to Jens, we were adding more and more preprocessors to make Pandoc happy and postprocessors to render it correctly. This is the latest version of the fork. Later, we reworked it completely to a new project, which we'll publish soon.

But if we were doing it again, we would get rid of Pandoc completely. The preprocessing is done by partial rendering using code adapted from Redmine / Redcloth3. The amount of the code is comparable to normal rendering and invoking Pandoc just makes it slow.

Pandoc is a great tool, but this use case is just too specific for a universal format converter.

So the message is just: a good converter exists for Redmine. :)

Hi, thanks for the feedback!

Nice catch about the rendering of Markdown files, that indeed should be switched to commonmark as well. Also great work on the Migrator :)

General use of HTML Pipeline

It would really make sense to use HTML pipeline for Textile rendering as well, as it will open up a lot of options for refactoring and sharing code (i.e. for HTML sanitization) between the different pipelines via filters. I intentionally stopped before doing this to gauge interest and get commonmark in as quickly as possible. We can (and imho should) rework the Textile rendering at a later point, as well as do some cleanup by moving rendering related code from helpers to the pipeline. The same could be done to the Redcarpet based markdown formatter if we decide to still keep that around at this time.

Configuration

For the sake of not overcomplicating this already complex patch I went with what I think are sensible defaults. I have no idea why Github is so inconsistent with their line break rendering and would definitely stay away from doing the same.

Introducing a way for configuring the rendering pipeline through plugins makes sense, maybe as part of the refactoring when we move over all formatters to the pipeline?

Regarding the task lists - to be honest I left this out since at Planio this would collide with the checklists plugin we have as a standard feature. Also this would require additional code to handle the checking / unchecking of the checkboxes in the rendered text (everywhere markdown is rendered) again making the patch more complex. I feel like this also really only makes sense in issue descriptions and (maybe) wiki pages. So this would be a case where we need slightly different pipeline config depending on the context.

Oh my this patch would be so nice. I know its always hard to say but is there any roadmap when this patch is available in an official release ?

Hans Riekehof wrote in #note-15:

[...] I know its always hard to say but is there any roadmap when this patch is available in an official release ?

Given all the parameters (size/scope/state/complexity of the patch, current Redmine release cycle, current issue scheduling) I'd say not earlier than before the end of 2020. However, this does not mean that it won't be available on the Redmine trunk earlier.

NB: Please keep this issue clean, focused, on-topic and free from superfluous comments...

Thank you for posting the patch.

I think we should remove Redcarpet instead of adding the second Markdown formatter. Although I understand that CommonMaker is not fully compatible with Redcarpet, having two different Markdown formatter introduces some problems.

• It is confusing. Maybe some users cannot understand why Redmine has two Markdown formatter and cannot determine which they should use
• Consumes more memory

Someday after adding support for CommonMaker, we need to remove Redcarpet. In my opinion, when the time we add CommonMaker is the best chance to remove Redcarpet. Replacing Redcarpet with CommonMaker is a simple and understandable story for everyone.

I agree regarding complexity / confusion of users. So in order to move this forward, should I proceed to change the patch so it directly replaces Redcarpet with CommonMarker for the markdown formatting?

Jens Krämer wrote:

I agree regarding complexity / confusion of users. So in order to move this forward, should I proceed to change the patch so it directly replaces Redcarpet with CommonMarker for the markdown formatting?

I'm my opinion, it is a good move to replace Redcarpet with CommonMarker, but I don't think that we can do it without helping users to migrate their existing content. Gitlab did this kind of migration and they had a very nice process: https://about.gitlab.com/blog/2019/06/13/how-we-migrated-our-markdown-processing-to-commonmark/

Well Gitlab is a hosted platform with paying customers, so they had to be extra careful. I don't know if/how that gradual release of commonmark also happened for users of their opensource version on their own servers. From reading that article it appears to me that they did not encounter many issues with existing content since they do not mention having done any automated conversions of Redcarpet data to commonmark.

It would certainly up complexity quite a bit if we were to add support for different formatters at the same time (i.e., rendering old content with RedCarpet and only new content with CommonMarker)...

Gitlab's diff tool could indeed be used as inspiration for a plugin that people could install before upgrading that could give them an idea about what percentage of their contents will render differently at all if they were to upgrade. I wouldn't build an automated converter however - it's really just few corner cases and chances are such a tool breaks more than it fixes.

Marius BALTEANU wrote:

I'm my opinion, it is a good move to replace Redcarpet with CommonMarker, but I don't think that we can do it without helping users to migrate their existing content.

• Substantial part of typical rich text contents is quite messy. Users are usually not familiar with the syntax details and they often copy&paste pieces of code, error messages, etc. without enclosing them to code blocks.
• Users often use familiar (i.e. CommonMark) constructs without paying attention to how they exactly render.
• Redmine did breaking changes in text formatting even before. E.g. until 3.4.7, users can write spaces before list items in Textile. In Markdown, superscript syntax was just introduced at some moment and introduction of underline using underscores could have been a surprise too.

I've written a few more thoughts on this topic here.
Formally speaking, conversion will improve things only if the currently rendered output better reflects what was intended to be written than input interpreted as GFM. Which is not the case in my experience. And transforming badly rendered Markdown to GFM would push the rendering problems to the text source.

Disclosure: I have this opinion despite of being the author of a tool which can do the conversion job. :)

Conversion tool:
We've published the redmine_reformat plugin.
It should do a good job for Textile -> Markdown conversion - tested on 250k strings. And it can be also used on common_mark-patched Redmine right away.

The RedcarpetMD -> GFM conversion can be done as MD -> patched Redmine's textilizable() -> HTML -> external Turndown microservice -> GFM (see examples if interested).
Our external Turndown based microservice is still in development and we are hoping for getting close to passing backtranslation tests (MD -> HTML -> MD). Which is quite a complex task and even bulletproof GFM escaping took almost 1000 lines of Javascript source code.
Having said this, I can only repeat that even a RedcarpetMD -> GFM converter with 100% rendering match would be hardly beneficial to most users.

Sorry for another longer post and hope it helps in decision making. :)

The previous post was a little biased, as we were using patched Redmine's configuration of Redcarpet. The biggest deal in the transition would be the hard_wrap option. Also _underline_ will change meaning to emphasis, and ^superscript^ will not work. I'm still convinced that any eventual conversion would do the best job if it just handles these particular issues at the text source level. We could try to make such converter if it were considered as a dealbreaker.

I'd discourage conversions based purely on pandoc - including the Gitlab's diff tool, which is even not finished according to its source code. :)

+1

I like this patch a lot, too, and agree that the HTML Pipeline will make existing code much nicer, modular and easier to maintain.

I would also vote for replacing the current markdown parser.

About the compatibility issues:

- hard_wrap: Commonmarker has a render option called HARDBREAKS which has the same effect (and this also seems to be the standard behaviour on GitHub)

- underline and superscript could just be replaces by their direct HTML equivalents (ins and sup) in the text source

GitHub uses HARDBREAKS only for rendering markdown in issues and similar (more short form) content. Wiki pages and markdown documents in repositories are rendered without hard line breaks.

Generally not using hard breaks is more sensible if you intend to make your content render properly on a variety of screen widths since it allows paragraphs to flow freely and lines will only end at the end of the available space and not where the original author decided to put a line ending. As such, it is also more important in long form content, less so in shorter snippets of text where it won't make too much of a difference in terms of readability.

I would prefer to not just continue with forcing hard breaks onto all users of Redmine, but let users choose. It's important to not only think about existing content here, but also what would be the optimum for new installations.

We could introduce a global setting (i.e. in configuration.yml) where the person setting up a Redmine installation can select either the old behavior (hard breaks everywhere), no hard breaks, or the Github approach with hard breaks in issues and comments but nowhere else. I do not know the exact reasoning that led Github to their decision to have inconsistent hard break rendering across their site, but it might have to do with having content flowing into issues / comments from emails (which is the case in Redmine as well), and/or with avoiding surprises for users (issue creators / commenters) who are unfamiliar with the way markdown line endings work by default.

To handle the issue with incoming emails (which rarely are written with Markdown in mind and therefore often render poorly without hard line breaks turned on) at Planio, we currently enforce hard breaks in content from emails by appending two spaces to each line of text that goes from an email into an issue or comment. This would not be necessary if we had different rendering modes, i.e. hard breaks on in issues / comments, hard breaks off elsewhere. The current architecture however doesn't really allow for this, the markdown formatter lacks context information. It would be easy to define two different markdown formatting pipelines, one with hard breaks, one without, but still the text formatting view helper has to get some context so further down the chain can be decided which pipeline to use.

Jens Krämer wrote:

Well Gitlab is a hosted platform with paying customers, so they had to be extra careful. I don't know if/how that gradual release of commonmark also happened for users of their opensource version on their own servers. From reading that article it appears to me that they did not encounter many issues with existing content since they do not mention having done any automated conversions of Redcarpet data to commonmark.

It would certainly up complexity quite a bit if we were to add support for different formatters at the same time (i.e., rendering old content with RedCarpet and only new content with CommonMarker)...

Gitlab's diff tool could indeed be used as inspiration for a plugin that people could install before upgrading that could give them an idea about what percentage of their contents will render differently at all if they were to upgrade. I wouldn't build an automated converter however - it's really just few corner cases and chances are such a tool breaks more than it fixes.

Sorry for my late reply on this.

I thought more to an easy way for users to find the content that must be updated, not to fix it automatically. Anyway, in the meantime, I saw Martin Cizek plugin that has some features that can help users with this change. Even if we do not provide any help for users, I'm still in favour of this move because RedCarped project is almost dead and, also, I think we can improve a lot the actual code if we start use the html_pipeline gem.

Jens, what do you think if we try to plug the html_pipeline gem to wiki formatting (both Textile and CommonMark)? I will be very happy to work on moving the current regexes from ApplicationHelper#parse_redmine_links and other methods that currently do content parsing to html pipeline filters. In the end, we would have almost the same pipeline, with only one difference, Textile or CommonMark filter.

Hey guys, although Marius already noticed something being cooked, let me announce it officialy now.

redmine_reformat now provides the MarkdownToCommonmark converter, which treats the biggest Redcarpet and GFM differences, i.e. hard line wraps, underscore _underlines_ and caret ^superscripts. It works solely by editing source markup, so it does not have the adverse effects of "full conversion" I mentioned above. On the other hand, it involves Markdown parser and recognizes Macros when locating places to edit, so it should be decently precise.

More info on MarkdownToCommonmark here. Basic usage is as easy as:
rake reformat:convert to_formatting=common_mark

Jens Krämer wrote:

global setting [...] select either the old behavior (hard breaks everywhere), no hard breaks, or the Github approach with hard breaks in issues and comments but nowhere else.

+1. And actually one of the secret reasons for creating the converter was a fear that the migration issue could be "solved" just by hardcoding the HARDBREAKS setting.

As for the user settings, the plugin already supports migration to mixed format setups. It can choose different conversion chains per object type. The configs for all three mentioned options are described here.

Jens Krämer wrote:

I wouldn't build an automated converter however

Sorry :)

it's really just few corner cases

Actually the hard breaks can be quite a pain.

chances are such a tool breaks more than it fixes.

...unless there is an approach that cannot break things by design. Bets accepted. :)

If there is anything we can do to increase your confidence, let me know.

We have also built Docker images including the common_mark formatting: orchitech/redmine-gfm.

It's based on official Docker images and the patches attached to this ticket. Can be used to try it out, but I consider it production-ready. Without warranty indeed.

• We introduce this as third text formatting option in 5.0.0.
• We mark the existing Markdown as deprecated in the same version.
• We remove the existing Markdown version in a future version (5.2.0 or maybe even 6.0)?

In this way, users will have enough time to update their content and make the transition to commonmark smoother.

Marius BALTEANU wrote:

What do you think about the following plan for this issue?
...

I really like this approach. IMHO making the transition while getting usual stuff done in production is a good option for many users.

Martin Cizek wrote:

We have also built Docker images including the common_mark formatting: orchitech/redmine-gfm.

Announcement for those who want to check out the CommonMark formatter:
After releasing Redmine 4.2, our Docker images orchitech/redmine-gfm were automatically released for Redmine 4.2 as well.

Marius BALTEANU wrote:

What do you think about the following plan for this issue?

It totally makes sense to release the new format ASAP and have a transition period.

Are you aware of any tasks that need to be done before the patches are applied (without removing the Redcarpet format)? I'd be happy to help... By now, we have tested it quite extensively and provided the incremental patches attached to this ticket. I actually don't see any reason why it cannot be added in e.g. Redmine 4.3. Especially when our automatic CI/CD was able to release Redmine-GFM 4.2 without any human touch. :-)

However, there is another thing that might deserve to come with a major release - it's the transition to HTML::Pipeline, which might or might not be released together with Redcarpet removal. I created #35035 for it (please feel free to link it to this issue).

• We introduce this as third text formatting option in 4.3.0 (or any release that comes soon). It can be marked as experimental for the very first release if there is not enough confidence.
• We refactor the text formatting to HTML::Pipeline in 5.0.0
• We mark the existing Markdown as deprecated in the same version - thus it doesn't have to be subject of full refactoring to HTML::Pipeline.
• We remove the existing Markdown version in a future version (5.2.0 or maybe even 6.0)?

In this way, users will have even more time to update their content, they will also have extra motivation to do it (benefits described in #35035) and Redmine text formatting subsystem will be pushed in the right direction at the same time.

What do you think?

great to see this moving forward :)

I think the addition of CommonMark as an "experimental" third formatting really soon is a good idea. Since it would be an entirely new feature without breaking changes this could be done in 4.3 or 5.0 (in case no 4.3 is planned).

Refactoring Textile to use the HTML pipeline shouldn't be something any user would notice, so we could do this anytime after the pipeline is in. I'd be happy to help with that.

The original Markdown formatter should be marked deprecated the moment we remove the "experimental" label from the CommonMark formatter (probably this would happen in the next minor release after it's introduction, depending on any issues with it that might come up). Since that will then most probably be a 5.x, we should imho wait until 6.0 before finally removing it. Users don't like changes that require some migration / effort on their part being forced onto them ever, so I think we should not rush it.

That however means we'll have the 'legacy markdown' around for quite some time and it might pay off to integrate it in the HTML pipeline as well in the same way we will do it with Textile. Otherwise we would have to carry around two versions of all the related wiki formatting helpers that we plan to move from ApplicationHelper to the pipeline until then. This would also make it trivial to later push it into a plugin for people who don't want to migrate or switch to commonmark for whatever reason at all.

Jens Krämer wrote:

Refactoring Textile to use the HTML pipeline shouldn't be something any user would notice, so we could do this anytime after the pipeline is in. I'd be happy to help with that.

Maybe if you don't mind reviewing/elaborating on #35035? I've created it to give some momentum to the pipeline adoption, but the description is quite abstract and maybe even naive. So your further clarification would improve credibility of the ticket. :)

Jens Krämer wrote:

That however means we'll have the 'legacy markdown' around for quite some time and it might pay off to integrate it in the HTML pipeline as well in the same way we will do it with Textile.

Makes sense. Looking at #32424-32 + #32424-36 + #32424-37, I guess we are in agreement ... Marius?

Thank you all for your valuable work in providing this patch!
I have issues with non http links using protocols like ssh or scp. We use lot's of them in our wiki pages. Following the commonmark spec for Autolink something like <ssh://user@test.mycompany.local> should work but actually does not.
Is this an issue of the library you embed or is there a work around to create links using other schemes than http?

I found your protocol whitelist definition in lib/redmine/wiki_formatting/common_mark/sanitization_filter.rb but that seems not to work as expected. Even when trying to use ftp which is in your whitelist no link will be rendered

[test](ftp://test.mycompany.com)  
ftp://test.mycompany.com

found the issue:
in 0001-CommonMark-Markdown-text-formatter.patch you used :a instead of "a" in line 522
changing to

         whitelist[:protocols]["a"] = [

I would suggest to either add common protocols like 'scp', 'ssh', 'tel', 'call' or make the protocols configurable not to force patching if someone needs other protocols.

Heiko Robert wrote:

whitelist[:protocols]["a"] = [

Please be warned, this would allow everything, including user-supplied javascript.
The correct fix with the allowlist approach would be:

          whitelist[:protocols]["a"]["href"] = [

add common protocols like 'scp', 'ssh', 'tel', 'cal

It seems that the current Textile formatter only forbids 'javascript'. And I've checked that e.g. GitLab only removes unsafe schemes too. So we probably should go that way. I'll try to prepare an incremental patch for it.

or make the protocols configurable not to force patching

This would probably better fit in #35035 (if it was still relevant). To be honest, I'd happy to see at least anything merged anytime soon.

Martin Cizek wrote:

The problem was that the overly restricted href URL issue was actually copied from the old Markdown implementation (already reported in #32766). We should distinguish URI safety for automatically loaded resources (like images) and harmlessness to just render a link.

• Old Markdown uses Redmine::Helpers::URL#uri_with_safe_scheme? for both decisions.
• Textile uses Redmine::Helpers::URL#uri_with_safe_scheme? for images and a hardcoded test ref.downcase.start_with?('javascript:') for link hrefs.

That's why I've introduced Redmine::Helpers::URL#uri_with_link_safe_scheme? to determine safety of only-rendered, user-activated links. The change could have been briefer, but I've used a Sanitize transformer, as Sanitize authors suggested to us.

I like that approach.

Thanks for pointing out the bug with the allow list and the correct form `whitelist[:protocols]["a"]["href"] = ...`.

Marius, I'd like to ask you to asses and merge this ASAP to master / trunk. Jens did an excellent job and I tried hard too, the result apparently meets the quality level set in Redmine and even the 4-eyes principle apparently happened.

On one hand, the current implementation is outdated, with many unresolved and/or unresolvable issues and e.g. #32563 might be even a security vulnerability. The implementation was indeed good at the time it was written, but both the software components and user expectations have evolved. It's pretty safe to assume the user expectations being set by GitHub and GitLab, but even JIRA is currently moving towards CommonMark flavour of Markdown.

On the other hand, not having the new implementation in the repo means that I can't contribute to related things (or I have to create patches against "expected future state" - which I do and it's quite frustrating). It also means that the strategic work on HTML::Pipeline is now blocked.

Any feedback would be greatly appreciated and please do make use of the contributor power. :)

Martin Cizek wrote:

Marius, I'd like to ask you to asses and merge this ASAP to master / trunk. Jens did an excellent job and I tried hard too, the result apparently meets the quality level set in Redmine and even the 4-eyes principle apparently happened.

On one hand, the current implementation is outdated, with many unresolved and/or unresolvable issues and e.g. #32563 might be even a security vulnerability. The implementation was indeed good at the time it was written, but both the software components and user expectations have evolved. It's pretty safe to assume the user expectations being set by GitHub and GitLab, but even JIRA is currently moving towards CommonMark flavour of Markdown.

On the other hand, not having the new implementation in the repo means that I can't contribute to related things (or I have to create patches against "expected future state" - which I do and it's quite frustrating). It also means that the strategic work on HTML::Pipeline is now blocked.

Any feedback would be greatly appreciated and please do make use of the contributor power. :)

I agree with you, we should have this committed as soon as possible, but unfortunately, it's not only my decision, we need to have green light from Jean-Philippe about this and in general, about all major changes. I will do my best to push this forward, I think I can review the changes in May. As a side note, I don't even have rights to commit code to Redmine core :)

Awesome, thanks for your work to bring it all together :)

It is great that a new, more expressive Markdown formatter will be available in the next version of Redmine. Thank you to everyone involved in the development of this patch.

However, there is only one concern. That is HARDBREAKS.

• As mentioned in #32424#note-26, issues created from incoming emails often have layout problems
• Some users tend to use hard breaks a lot when writing issues. It is difficult to convince them to learn the concept of paragraphs and how to break lines in CommonMark. This is because they are not writing Markdown, they are just writing text
• As far as I know, in Japan, hard breaks tend to be used a lot in issues, and if hard breaks are not available, it can cause a lot of frustration for some of them

For organizations that need hard breaks, we hope that one of the following measures will be taken:

• A setting to switch the hardbreak mode on /settings page
• Add another option "CommonMark Markdown with hard breaks" to the "Text formatting" setting

If there is no option to enable hard breaks, I am concerned that this will be a big problem in Japan.

I think we should conclude over the HARDBREAKS before committing this.

• using HARDBREAKS or not is still CommonMark compliant, the spec covers both implementations.

A renderer may also provide an option to render soft line breaks as hard line breaks.

• as Jens mentioned, the real advantage of not using HARDBREAKS is on long form content where the user can easily control when to break the line or not.
• as Go Maeda mentioned in his last comment, forcing the users to learn a special syntax (two spaces or \) for such a simple operation (line break) could be a real issue because Redmine, compared with Gitlab/Github, is not used only on tech side. Even in a tech company, the users who work on operational teams (ex: sales, finance, marketing) will not easily learn this special syntax when they just want to create a simple ticket. For iOS/OSx users, adding two spaces can be a real trick because of the feature that replaces the two spaces with a dot.

• by default, to support HARDBREAKS using the regular line break.
• have an option in configuration.yml to disable this behaviour and use the special syntax to line breaks. I'm not in favour of having a mix behaviour as Github does.

If the feature of controlling when to break the line is so useful, we can do a Redmine flavour and add a special syntax that continues the line instead of breaking (something like "&" or "&&"). I'm not sure if it's the best ideea, but it's an option.

Marius BALTEANU wrote:

My proposal is the following:

• by default, to support HARDBREAKS using the regular line break.
• have an option in configuration.yml to disable this behaviour and use the special syntax to line breaks. I'm not in favour of having a mix behaviour as Github does.

I agree. Enabling HARDBREAKS by default removes the barrier to switching from RedCarpet to CommonMark.

On that basis, I have a few suggestions:

1. With HARDBREAKS enabled by default, the new CommonMark based Markdown is highly compatible with the current RedCarpet based Markdown. Maybe we can give the CommonMark base the name Markdown "Markdown" (Setting.text_formatting: markdown) and rename the current RedCarpet based Markdown to "Markdown (classic)" (Setting.text_formatting: markdown_redcarpet). This means users will start using CommonMark without being aware of it after upgrading to Redmine 5.0. Only people who experience some issue will switch the setting to "Markdown (classic)". Or we can completely remove RedCarpet in Redmine 5.0, as we switched the syntax highlighter from CodeRay to Rouge (JPL didn't want keeping two libraries. See #24681#note-23).
2. I think it will be even better if an admin can change the HARDBREAKS behavior in the /admin/settings page instead of configuration.yml. This is because Redmine admins don't always have access to configuration files on a Redmine server (e.g. companies which have a dedicated information system department, users using SaaS based Redmine)

Go MAEDA wrote:

I agree. Enabling HARDBREAKS by default removes the barrier to switching from RedCarpet to CommonMark.

On that basis, I have a few suggestions:

1. With HARDBREAKS enabled by default, the new CommonMark based Markdown is highly compatible with the current RedCarpet based Markdown. Maybe we can give the CommonMark base the name Markdown "Markdown" (Setting.text_formatting: markdown) and rename the current RedCarpet based Markdown to "Markdown (classic)" (Setting.text_formatting: markdown_redcarpet). This means users will start using CommonMark without being aware of it after upgrading to Redmine 5.0. Only people who experience some issue will switch the setting to "Markdown (classic)". Or we can completely remove RedCarpet in Redmine 5.0, as we switched the syntax highlighter from CodeRay to Rouge (JPL didn't want keeping two libraries. See #24681#note-23).

I don't think that it's the same case with the syntax highlighter. In this case, there are certain situation where the content must be prepared for common_mark before migration. I'm more comfortable to stick with the initial plan that will give us time to prepare the migration: introduce this an experimental feature, deprecate old markdown, migrate markdown to common_mark in a later version.

1. I think it will be even better if an admin can change the HARDBREAKS behavior in the /admin/settings page instead of configuration.yml. This is because Redmine admins don't always have access to configuration files on a Redmine server (e.g. companies which have a dedicated information system department, users using SaaS based Redmine)

We can add this in admin/settings, but changing the setting will still require an application restart in order to become effective.

Marius BALTEANU wrote:

I don't think that it's the same case with the syntax highlighter. In this case, there are certain situation where the content must be prepared for common_mark before migration. I'm more comfortable to stick with the initial plan that will give us time to prepare the migration: introduce this an experimental feature, deprecate old markdown, migrate markdown to common_mark in a later version.

Your opinion makes sense.

1. I think it will be even better if an admin can change the HARDBREAKS behavior in the /admin/settings page instead of configuration.yml. This is because Redmine admins don't always have access to configuration files on a Redmine server (e.g. companies which have a dedicated information system department, users using SaaS based Redmine)

We can add this in admin/settings, but changing the setting will still require an application restart in order to become effective.

I didn't know that a restart is required. If that is the case, I think put the setting in configuration.yml as you suggested is the best way. Some Redmine admins who don't have access to the underlying OS cannot restart Redmine by themselves.

Attached a WIP patch for the configuration setting and now we have both options available for testing. Go Maeda, Jens Krämer, Martin, what do you think about this?

Jens Krämer wrote:

To handle the issue with incoming emails (which rarely are written with Markdown in mind and therefore often render poorly without hard line breaks turned on) at Planio, we currently enforce hard breaks in content from emails by appending two spaces to each line of text that goes from an email into an issue or comment.

Do you think it's possible to extract this change as a patch? I think we should do the same thing.

Rails 6.1 and Redmine 5.0.0 supports minimum Ruby 2.5, but the CommonMarker latest version (0.22.0) supports minimum Ruby 2.6.

We should drop support for Ruby 2.5 or we should stick with commonmarker version 0.21.0 if ruby is 2.5? Note that Ruby 2.5 is EOL since March this year.

I would be fine with requiring Ruby 2.6 for Redmine 5.0.

Some notes here:

• Debian 11 Bullseye is going to ship with Ruby 2.7 on August 14.
• Ubuntu >= 20 ships with Ruby 2.7
• Debian 10 (Buster) and Ubuntu 18.04 (bionic) still ship Ruby 2.5. Users would have to either use a custom Ruby or stay on Redmine 4.2
• CentOS 8 / Redhat 8 can install Ruby 2.7 with dnf.
• For Windows, there are usable installers for current Rubies.

With this change, we would thus cut off Debian 10 and Ubuntu 18.04 with their respective system-default Rubies. They can install a custom Ruby with rvm / rbenv / ruby-install though.

Thanks Holger for you reply, let's move the discussion about Ruby 2.5 support to #31128 (I've reopen it and I added our comments).

It's really nice to see the progress of this issue. :thumbsup:

• 5.0.0
  • Introduce the new formatter as experimental and point to users to Martin's amazing redmine_reformat.
• 5.1.x-5.x.x
  • Remove experimental flag and direct users to redmine_reformat.
• 6.0.0
  • Deprecate the old RedCarpet-based formatter and push users to redmine_reformat.
• 6.1.0-6.x.x
  • Finally remove the old RedCarpet-based formatter.

Marius BALTEANU wrote:

Jens (or some else from Plan.io team), being the original author of this patch, please let me know what do you think about the two solutions that I posted for controlling the behaviour of hardbreaks:

1. #32424#note-65: which allows you to control the settings from configuration.yml.
2. the attached patch which allows you to control the setting from the administration page.

I believe this setting really isn't something one would likely change more than once. Either it's decided when setting up a new instance and then kept as is, or it's set when converting the data in an existing instance to 'no hard breaks'. Therefore, I think it is perfectly fine to keep it exclusively in `configuration.yml`.

After we conclude on this, I would like to fix the incoming emails when the hardbreaks is disabled and to close this issue in the following two weeks.

At Planio we use something like this (diff against master, patching cleanup_body in MailHandler):

diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index fd2e25fb5..2a71d57cb 100644
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -638,6 +638,9 @@ class MailHandler < ActionMailer::Base
       regex = Regexp.new("^(\\p{Space}|>)*(#{ Regexp.union(delimiters) })\\p{Space}*[\\r\\n].*", Regexp::MULTILINE)
       body = body.gsub(regex, '')
     end
+    if Setting.text_formatting == "common_mark" 
+      body = Redmine::WikiFormatting::CommonMark::AppendSpacesToLines.(body)
+    end
     body.strip
   end

diff --git a/lib/redmine/wiki_formatting/common_mark/append_spaces_to_lines.rb b/lib/redmine/wiki_formatting/common_mark/append_spaces_to_lines.rb
new file mode 100644
index 000000000..34ee1553f
--- /dev/null
+++ b/lib/redmine/wiki_formatting/common_mark/append_spaces_to_lines.rb
@@ -0,0 +1,13 @@
+module Redmine
+  module WikiFormatting
+    module CommonMark
+      class AppendSpacesToLines
+        def self.call(string)
+          string&.gsub(/(?<!  |\\)(\r?\n)/, "  \\1")
+        end
+      end
+    end
+  end
+end

The check for the hard breaks setting would still have to be added of course, instead of just checking for common_mark formatting. It might be worth to think about putting this into a generic postprocessing step implemented in the WikiFormatting namespace, where we then could delegate to the text format specific implementation. Something like body = Redmine::WikiFormatting.post_process_incoming_mail_body(body), maybe?

Hi guys, I couldn't watch this for a while. I like the configurable approach for hardbreaks. Thank you all for finalizing this.

P.S. Will update the follow-up tickets+patches ASAP. And I guess I'll have to keep hands on `redmine_reformat`. :)

Working on #35742, I didn't find an easy way to add a toolbar icon only for common_mark formatter because heads_for_wiki_formatter from common_mark/helper.rb inherits the heads_for_wiki_formatter from markdown/helper.rb and both formatters share the same markdown.js file. I decided to make each formatter independent even if this means a little bit more duplicated code.

this is great we might migrate to this soon on our instance