Redmine

Go MAEDA wrote in #note-1:

Thank you for the proposal to add an "Underline" button to the CommonMark Markdown toolbar.

However, I think it is better to use <u> instead of <ins>, as in Redcarpet-based Markdown. This is because <ins> is semantically intended to represent inserted text, so using <ins> only to express underline is not semantically appropriate.

I have updated the patch to use the <u> element accordingly.

Thank you for the correction and explanation.
But, I am a little concerned about the `ins` in the comments, including other text formatting. Why?

I investigated why the comment // ins is used for the "Underline" button in markdown.js.

First, in Textile, which was the only available formatter before Redcarpet-based Markdown was implemented in Redmine 2.5.0, the <ins> tag is used to represent underlined text. Therefore, it makes sense for the comment // ins to be used for the "Underline" button in textile.js (source:trunk/app/assets/javascripts/jstoolbar/textile.js@22937#L29).

However, in Markdown based on Redcarpet, the <u> tag is used for underlined text. Despite this, the comment in markdown.js still says // ins (source:trunk/app/assets/javascripts/jstoolbar/markdown.js@22937#L29). It would be more appropriate for it to say // u. It is likely that when the underline button was added to the Markdown toolbar in r17385, the comment // ins from textile.js was copied over without being changed.

Therefore, I suggest proceeding as follows:

• For the CommonMark Markdown toolbar, follow the implementation of Redcarpet-based Markdown and use the <u> tag to represent underlined text, as I proposed in 40939-v2.patch.
• Change the comment // ins in 40939-v2.patch to // u.
• Change the existing comments in markdown.js to // u to match the HTML output by Redcarpet.

Thank you Mr. Go MAEDA.

I have understood the historical context in which the //ins comment was added.
In addition, I think it would be better to change jsToolBar.prototype.elements.ins to jsToolBar.prototype.elements.u.
What do you think?

diff --git a/app/assets/javascripts/jstoolbar/common_mark.js b/app/assets/javascripts/jstoolbar/common_mark.js
index f9e5ba7..5c3627a 100644
--- a/app/assets/javascripts/jstoolbar/common_mark.js
+++ b/app/assets/javascripts/jstoolbar/common_mark.js
@@ -26,6 +26,16 @@ jsToolBar.prototype.elements.em = {
   }
 }

+// u
+jsToolBar.prototype.elements.u = {
+  type: 'button',
+  title: 'Underline',
+  shortcut: 'u',
+  fn: {
+    wiki: function() { this.singleTag('<u>', '</u>') }
+  }
+}
+
 // del
 jsToolBar.prototype.elements.del = {
   type: 'button',
diff --git a/app/assets/stylesheets/jstoolbar.css b/app/assets/stylesheets/jstoolbar.css
index 6870261..38c70c1 100644
--- a/app/assets/stylesheets/jstoolbar.css
+++ b/app/assets/stylesheets/jstoolbar.css
@@ -113,6 +113,9 @@
 .jstb_ins {
     background-image: url(/jstoolbar/bt_ins.png);
 }
+.jstb_u {
+    background-image: url(/jstoolbar/bt_ins.png);
+}
 .jstb_del {
     background-image: url(/jstoolbar/bt_del.png);
 }