Feature #34863
closedChange default text formatter for new installations from textile to common_mark
Added by Marius BĂLTEANU over 3 years ago. Updated 10 months ago.
0%
Description
Nowadays, markdown is much more used and known compared with textile. Also, in Redmine, Markdown has better support.
My proposal is to change the default setting from textile to markdown.
Files
Related issues
Updated by Marius BĂLTEANU over 3 years ago
- Subject changed from Change default text formatter for new installation from textile to markdown to Change default text formatter for new installations from textile to markdown
Updated by Mizuki ISHIKAWA over 3 years ago
+1
Since it is not possible to change the text formatter setting for each project in Redmine, there is also the problem that it is difficult to change to Markdown once you start using it with Textile.
I think it's better to use the commonly used Markdown as the default.
Updated by beko akabeko over 3 years ago
+1
Markdown is used in many text systems (GitHub, Slack, ...etc). Therefore, setting this as the default will improve portability.
Updated by Moritz Poldrack over 3 years ago
+1
Since it is not possible to change the text formatter setting for each project in Redmine, there is also the problem that it is difficult to change to Markdown once you start using it with Textile.
I think it's better to use the commonly used Markdown as the default.
it's possible using grep
, sed
, and pandoc
. but it's a real PITA
Updated by Bernhard Rohloff over 3 years ago
+1
Markdown is also the better choice for the 'average guy' and has easier to adopt by non IT people.
Updated by Go MAEDA over 3 years ago
- File 0001-Explicitly-specify-text-formatting-in-the-test-suite.patch 0001-Explicitly-specify-text-formatting-in-the-test-suite.patch added
- File 0002-Change-the-default-text-formatting-to-Markdown.patch 0002-Change-the-default-text-formatting-to-Markdown.patch added
- File 0003-Change-the-text-formatting-of-fixtures-to-Markdown.patch 0003-Change-the-text-formatting-of-fixtures-to-Markdown.patch added
The attached patches change the default text formatting to Markdown.
Updated by Go MAEDA over 3 years ago
- Target version set to 5.0.0
Setting the target version to 5.0.0.
Updated by Marius BĂLTEANU almost 3 years ago
- Target version changed from 5.0.0 to Candidate for next major release
I think the best option is to change to CommonMark when we will remove the experimental flag.
Updated by Go MAEDA over 2 years ago
- Related to Patch #35952: Explicitly specify text formatting in the test suite added
Updated by Marius BĂLTEANU over 2 years ago
Now that CommonMark feature was added to the core, for sure we should not set the old Markdown as default, but at the same time, it's pity to keep textile as default.
Maybe is better to set CommonMark even is it marked as experimental? Or even remove the experimental flag?
Updated by Go MAEDA over 2 years ago
- Target version changed from Candidate for next major release to 6.0.0
Updated by Go MAEDA over 2 years ago
- Target version changed from 6.0.0 to 5.1.0
Updated by Go MAEDA over 2 years ago
- Blocked by Patch #36807: Remove CommonMark experimental flag and mark as deprecated the RedCarpet Markdown added
Updated by Marius BĂLTEANU over 2 years ago
- Subject changed from Change default text formatter for new installations from textile to markdown to Change default text formatter for new installations from textile to common_mark
Updated by Marius BĂLTEANU over 2 years ago
- File 0001-Set-common_mark-as-default-text-formatting.patch 0001-Set-common_mark-as-default-text-formatting.patch added
Here is the patch to set common_mark the default formatter.
Updated by Holger Just about 2 years ago
When changing the default formatter, we should ensure that the previous setting is definitely preserved. Often, the setting is stored in the database, but not necessarily. It can also be used with just the default from settings.yml
. Changing the setting just there will silently change the global text formatting setting during an upgrade.
As such, I believe we need a migration here that strictly adds the current setting as a database entry before changing this (rather important) default setting. This must take several cases into account:
- Upgrade - An existing Redmine with a
text_formatting
setting stored in the database - Upgrade - An existing Redmine with the default
text_formatting
from settings.yml without an explicitly stored setting - New Setup - An initial setup with an empty database.
A possible solution would be to perform the following two changes:
- Add a new migration which sets the
text_formatting
setting in the database to the static valuetextile
, only if there is no existing entry in the database.
This should take care of the upgrade paths to retain the previous (default or adapted) setting regardless of how we change the default insettings.yml
. - Update an existing old migration to explicitly set the
common_mark
setting to the database in the same commit as the update to thesettings.yml
. E.g. the source:trunk/db/migrate/017_create_settings.rb migration.
This should take care of the initial setup. As the new default setting is set to the DB early, the later new migration will no nothing in this case. Existing installation won't re-apply the changed initial migration.
As for the timeline, please let me quote my comment from #36807#note-5:
The default formatter is only relevant for new installations. I have no strong opinion on whether this requires a major release (6.0.0) or minor release (5.1.0). I have a slight hunch that it might be surprising in a patch release (5.0.x).
Updated by Marius BĂLTEANU about 2 years ago
- File 0001-Set-common_mark-as-default-text-formatting.patch 0001-Set-common_mark-as-default-text-formatting.patch added
Thanks Holger for your valuable feedback, I wasn't aware that the default settings are not stored in the db from the beginning.
I think the attached patch should do the trick:
1. Setting text_formatting
already stored in the database: it won't do anything and this is the expected behaviour
2. Setting text_formatting
doesn't exist in the database: it will run the migration and it will persist the Setting.text_formatting
in the db instead of the static textile
value. That means for existing installations it will insert textile
most probably (if the setting.yml file wasn't manually changed) and for new installations, it will insert @common_mark
.
What do you think? Am I missing any case?
Updated by Jens Krämer about 2 years ago
In an existing installation without a setting in the database, when applying the patch and then running the migration, Setting.text_formatting
will already return common_mark
with the changed default in settings.yml
.
I think the migration should set the value in the database literally to textile
(as Holger suggested) in case no setting is present, to persist the previous default.
(Edit: Of course in that case Holger's second point, changing the create_settings migration in the same commit to reflect the new default for fresh installations, is necessary as well)
Updated by Marius BĂLTEANU about 2 years ago
You're right Jens, I didn't see this point. Thanks both for your feedback on this.
I'm going to update the patches in the following days.
Updated by Marius BĂLTEANU almost 2 years ago
- File 0001-Set-common_mark-as-default-text-formatting.diff 0001-Set-common_mark-as-default-text-formatting.diff added
- for existing installations, it explicit save the "textile" value in the database
- for new installations, it set during the
017_create_settings.rb
migration the value toSetting.text_formatting
. - it adds a test to assert that the default value for new installations is "common_mark".
Jens, Holger, can you take a look on the patches, please? I would like a double check before committing this.
Updated by Marius BĂLTEANU almost 2 years ago
- Status changed from New to Resolved
I've committed the change, please let me know if there are any issues.
Updated by Go MAEDA almost 2 years ago
Since CommonMark is the default formatter now, commonmarker gem should not be optional. And since Redmine already does not support JRuby, there is no need to set common_mark to optional in consideration of JRuby.
diff --git a/Gemfile b/Gemfile
index bccf76491..2d15ba953 100644
--- a/Gemfile
+++ b/Gemfile
@@ -48,11 +48,9 @@ group :markdown do
gem 'redcarpet', '~> 3.5.1'
end
-# Optional CommonMark support, not for JRuby
-group :common_mark do
- gem "commonmarker", '0.23.4'
- gem 'deckar01-task_list', '2.3.2'
-end
+# CommonMark support
+gem "commonmarker", '0.23.4'
+gem 'deckar01-task_list', '2.3.2'
# Include database gems for the adapters found in the database
# configuration file
Updated by Go MAEDA almost 2 years ago
A test added in r21897 fails.
Error: SettingTest#test_default_text_formatting_for_new_installations_is_common_mark: NoMethodError: undefined method `value' for nil:NilClass assert_equal 'common_mark', Setting.find_by(:name => 'text_formatting').value ^^^^^^ test/unit/setting_test.rb:151:in `test_default_text_formatting_for_new_installations_is_common_mark' rails test test/unit/setting_test.rb:149
Updated by Marius BĂLTEANU almost 2 years ago
- Status changed from Resolved to Reopened
Updated by Ko Nagase almost 2 years ago
- File 0001-Add-settings-fixture-for-default-text_formatting.patch 0001-Add-settings-fixture-for-default-text_formatting.patch added
Go MAEDA wrote:
A test added in r21897 fails.
[...]
About the above common mark test error, I confirmed that test env database doesn't have settings table record for "text_formatting" => "common_mark", while development env database has the record.
I upload settings fixture patch for test env, so reviewing it is helpful.
(Update) GitHub CI before/after results are here:
Updated by Holger Just over 1 year ago
- Related to Feature #37365: Allow users to choose textile or markdown formatting added