Defect #32971
closed
New line between list items break a list
0%
Description
New line between list items break a list when I try to write ordered/unordered list with textile.
- Example
* list1 * list2 * list3 # list4 # list5 # list6
- In 3.2.1
- In 4.1.0
Files
Related issues
Updated by Yuichi HARADA about 4 years ago
- File 32971-fixed.patch 32971-fixed.patch added
I think the behavior has probably changed by applying r17603 ( Defect !#29756: \f or \v character in Textile markup may cause RegexpError exception ).
It could be resolved with the following patch.
diff --git a/lib/redmine/wiki_formatting/textile/redcloth3.rb b/lib/redmine/wiki_formatting/textile/redcloth3.rb
index 80e0a3626..ff4687f7e 100644
--- a/lib/redmine/wiki_formatting/textile/redcloth3.rb
+++ b/lib/redmine/wiki_formatting/textile/redcloth3.rb
@@ -1020,10 +1020,13 @@ class RedCloth3 < String
end
def flush_left( text )
- indt = 0
- if text =~ /^ /
+ if /(?![\r\n\t ])[[:cntrl:]]/.match?(text)
+ text.gsub!(/(?![\r\n\t ])[[:cntrl:]]/, '')
+ end
+ if /^ /.match?(text) && text.length > 1
+ indt = 0
unless text.empty?
- indt += 1 while text !~ /^ {#{indt}}[^ ]/
+ indt += 1 while text !~ /^ {#{indt}}\S/
end
if indt.nonzero?
text.gsub!( /^ {#{indt}}/, '' )
Updated by Jihyeon Gim about 4 years ago
Thanks for your help Yuichi! :)
This patch works gracefully well!
Thank you again for patching it :)
Updated by Go MAEDA about 4 years ago
- Related to Defect #29756: \f or \v character in Textile markup may cause RegexpError exception added
Updated by Go MAEDA about 4 years ago
- Target version set to Candidate for next minor release
Updated by Yuichi HARADA about 4 years ago
- File 32971-fixed-v2.patch added
Yuichi HARADA wrote:
I think the behavior has probably changed by applying r17603 ( Defect !#29756: \f or \v character in Textile markup may cause RegexpError exception ).
It could be resolved with the following patch.
[...]
Sorry, I found that String#lstrip! could do the same.
https://www.rubydoc.info/stdlib/core/String#lstrip!-instance_method
diff --git a/lib/redmine/wiki_formatting/textile/redcloth3.rb b/lib/redmine/wiki_formatting/textile/redcloth3.rb
index 80e0a3626..815f860e6 100644
--- a/lib/redmine/wiki_formatting/textile/redcloth3.rb
+++ b/lib/redmine/wiki_formatting/textile/redcloth3.rb
@@ -1020,15 +1020,7 @@ class RedCloth3 < String
end
def flush_left( text )
- indt = 0
- if text =~ /^ /
- unless text.empty?
- indt += 1 while text !~ /^ {#{indt}}[^ ]/
- end
- if indt.nonzero?
- text.gsub!( /^ {#{indt}}/, '' )
- end
- end
+ text&.lstrip!
end
def footnote_ref( text )
Updated by Yuichi HARADA about 4 years ago
Yuichi HARADA wrote:
Yuichi HARADA wrote:
I think the behavior has probably changed by applying r17603 ( Defect !#29756: \f or \v character in Textile markup may cause RegexpError exception ).
It could be resolved with the following patch.
[...]
Sorry, I found that
String#lstrip!could do the same.https://www.rubydoc.info/stdlib/core/String#lstrip!-instance_method
[...]
It did not work well with attachment:32971-fixed-v2.patch . The same problem occurred as in the previous situation.
Please delete attachment:32971-fixed-v2.patch .
Updated by Yuichi HARADA about 4 years ago
- File 32971-fixed-v3.patch 32971-fixed-v3.patch added
I fixed the 32971-fixed.patch because sometimes the flush_left method did not end.
diff --git a/lib/redmine/wiki_formatting/textile/redcloth3.rb b/lib/redmine/wiki_formatting/textile/redcloth3.rb
index 80e0a3626..c9bdd0aeb 100644
--- a/lib/redmine/wiki_formatting/textile/redcloth3.rb
+++ b/lib/redmine/wiki_formatting/textile/redcloth3.rb
@@ -1020,11 +1020,12 @@ class RedCloth3 < String
end
def flush_left( text )
- indt = 0
- if text =~ /^ /
- unless text.empty?
- indt += 1 while text !~ /^ {#{indt}}[^ ]/
- end
+ if /(?![\r\n\t ])[[:cntrl:]]/.match?(text)
+ text.gsub!(/(?![\r\n\t ])[[:cntrl:]]/, '')
+ end
+ if /^ +\S/.match?(text)
+ indt = 0
+ indt += 1 while !/^ {#{indt}}\S/.match?(text)
if indt.nonzero?
text.gsub!( /^ {#{indt}}/, '' )
end
Updated by Kevin Fischer about 4 years ago
How about adding a unit test to prevent future regressions of this conversion?
Updated by Yuichi HARADA about 4 years ago
Ji-Hyeon Gim wrote:
New line between list items break a list when I try to write ordered/unordered list with textile.
- Example
* list1 * list2 * list3 # list4 # list5 # list6
- In 3.2.1
I noticed when looking at source:trunk/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb#L148 that this nested list is probably wrong for Textile.
def test_nested_lists
raw = <<~RAW
# Item 1
# Item 2
** Item 2a
** Item 2b
# Item 3
** Item 3a
RAW
expected = <<~EXPECTED
<ol>
<li>Item 1</li>
<li>Item 2
<ul>
<li>Item 2a</li>
<li>Item 2b</li>
</ul>
</li>
<li>Item 3
<ul>
<li>Item 3a</li>
</ul>
</li>
</ol>
EXPECTED
assert_equal expected.gsub(%r{\s+}, ''), to_html(raw).gsub(%r{\s+}, '')
end
Updated by Yuichi HARADA about 4 years ago
- File 32971-fixed-v4.patch 32971-fixed-v4.patch added
Kevin Fischer wrote:
How about adding a unit test to prevent future regressions of this conversion?
Thank you for pointing it out.
I have added the following test to 32971-fixed-v3.patch .
diff --git a/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb b/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb
index 19128524e..2358ded58 100644
--- a/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb
+++ b/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb
@@ -171,6 +171,24 @@ class Redmine::WikiFormatting::TextileFormatterTest < ActionView::TestCase
</ol>
EXPECTED
assert_equal expected.gsub(%r{\s+}, ''), to_html(raw).gsub(%r{\s+}, '')
+
+ raw = <<~RAW
+ * Item-1
+
+ * Item-1a
+ * Item-1b
+ RAW
+ expected = <<~EXPECTED
+ <ul>
+ <li>Item-1
+ <ul>
+ <li>Item-1a</li>
+ <li>Item-1b</li>
+ </ul>
+ </li>
+ </ul>
+ EXPECTED
+ assert_equal expected.gsub(%r{\s+}, ''), to_html(raw).gsub(%r{\s+}, '')
end
def test_escaping
Updated by Kevin Fischer about 4 years ago
Tried it out. The test passed, so looks good to me!
Updated by Go MAEDA about 4 years ago
- Target version changed from Candidate for next minor release to 4.1.1
Setting the target version to 4.1.1.
Updated by Go MAEDA about 4 years ago
- Status changed from New to Resolved
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch. Thank you.