coradoc integration #89

xyz65535 · 2024-04-17T20:11:53Z

Metanorma PR checklist

Breaking changes (list related PRs)
Documentation update required (create task for this)
External dependency introduced (documentation update need)
Gem with native library introduced

hound · 2024-04-17T20:12:08Z

lib/reverse_adoc/converters/ol.rb

@@ -1,12 +1,36 @@
 module ReverseAdoc
  module Converters
    class Ol < Base
-      def convert(node, state = {})
+      def to_coradoc(node, state = {}) #FIXIT


Assignment Branch Condition size for to_coradoc is too high. [<11, 13, 4> 17.49/15]
Method has too many lines. [16/10]
Missing space after #.
Do not place comments on the same line as the def keyword.

hound · 2024-04-17T20:12:08Z

lib/reverse_adoc/converters/td.rb

@@ -1,9 +1,9 @@
 module ReverseAdoc
  module Converters
    class Td < Base
-      def convert(node, state = {})
+      def to_coradoc(node, state = {})


Assignment Branch Condition size for to_coradoc is too high. [<17, 23, 8> 29.7/15]
Cyclomatic complexity for to_coradoc is too high. [7/6]
Method has too many lines. [19/10]

hound · 2024-04-17T20:12:08Z

lib/reverse_adoc/html_converter.rb

      ReverseAdoc.config.with(options) do
-        result = ReverseAdoc::Converters.lookup(root.name).convert(root)
-        ReverseAdoc.cleaner.tidy(result)
+        result = ReverseAdoc::Converters.lookup(root.name).to_coradoc(root)


Useless assignment to variable - result.

lib/reverse_adoc/converters/aside.rb

lib/reverse_adoc/converters/blockquote.rb

ronaldtse · 2024-04-18T01:12:04Z

lib/reverse_adoc/converters/br.rb

      def convert(node, state = {})
-        " +\n"
+        Coradoc::Generator.gen_adoc(to_coradoc(node, state))


Why do we have to_coradoc and convert?

ronaldtse · 2024-04-18T01:12:28Z

lib/reverse_adoc/converters/drop.rb

@@ -1,6 +1,9 @@
 module ReverseAdoc
  module Converters
    class Drop < Base
+      def to_coradoc(node, state = {})
+        convert(node, state)


What does this do?

ronaldtse · 2024-04-18T01:19:50Z

lib/reverse_adoc/converters/p.rb

+        content = treat_children(node, state).strip
+        options = {}
+        options[:id] = id if id
+        options[:tdsinglepara] = true if state[:tdsinglepara]


I don't know what this is... let's delete it?

lib/reverse_adoc/converters/pre.rb

lib/reverse_adoc/converters/table.rb

ronaldtse · 2024-04-18T01:28:19Z

lib/reverse_adoc/converters/td.rb

@@ -13,8 +13,18 @@ def convert(node, state = {})
          node.at(".//p") && !singlepara
        style = "a" if adoccell
        delim = adoccell ? "\n" : " "


Delete all lines that are unused.

ronaldtse · 2024-04-18T01:29:45Z

lib/reverse_adoc/html_converter.rb

+      ReverseAdoc.cleaner.tidy(result)
+    end


Why do we need any cleaning after putting it into a Coradoc::Document? The to_adoc results should be perfect.

hound · 2024-04-19T05:29:35Z

lib/reverse_adoc/converters/text.rb

        end
      end
+      def convert(node, state = {})


Use empty lines between method definitions.

hound · 2024-04-19T05:29:35Z

lib/reverse_adoc/html_converter.rb

      end
    end
+


Trailing whitespace detected.

hound · 2024-04-19T05:29:35Z

lib/reverse_adoc/html_converter.rb

@@ -36,7 +36,7 @@

 module ReverseAdoc
  class HtmlConverter
-    def self.convert(input, options = {})
+    def self.to_coradoc(input, options = {})


Method has too many lines. [11/10]

hound · 2024-04-19T05:29:35Z

spec/lib/reverse_adoc/converters/video_spec.rb

-    expect(converter.convert(node)).to include "[[A]]\nvideo::example.mp3[options=\"loop\"]"
+  it 'converts video with full set of attributes' do
+    node = node_for("<video id='A' src='example.mp4' loop='loop'/>")
+    expect(converter.convert(node)).to include "[[A]]\nvideo::example.mp4[options=\"loop\"]"


Line is too long. [92/80]

hound · 2024-04-19T05:29:35Z

spec/lib/reverse_adoc/converters/video_spec.rb

-  it 'converts audio with full set of attributes' do
-    node = node_for("<audio id='A' src='example.mp3' loop='loop'/>")
-    expect(converter.convert(node)).to include "[[A]]\nvideo::example.mp3[options=\"loop\"]"
+  it 'converts video with full set of attributes' do


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

hound · 2024-04-19T05:29:35Z

lib/reverse_adoc/converters/img.rb

+        Coradoc::Document::Inline::Image.new(id: id,
+          title: title, src: src, alt: alt, width: width, height: height)
+      end
+      def convert(node, state = {})


Use empty lines between method definitions.

hound · 2024-04-19T05:29:36Z

lib/reverse_adoc/converters/img.rb

@@ -55,26 +55,25 @@ def copy_temp_file(imgdata)
        end
      end

-      def convert(node, state = {})
+      def to_coradoc(node, state = {})


Method has too many lines. [12/10]
Unused method argument - state. If it's necessary, use _ or _state as an argument name to indicate that it won't be used.

hound · 2024-04-19T05:29:36Z

lib/reverse_adoc/converters/math.rb

@@ -7,6 +7,9 @@
 module ReverseAdoc
  module Converters
    class Math < Base
+      def to_coradoc(node, state = {}) #FIXIT


Missing space after #.
Do not place comments on the same line as the def keyword.

hound · 2024-04-19T05:29:36Z

lib/reverse_adoc/converters/q.rb

-        "#{content[/^\s*/]}\"#{content.strip}\"#{content[/\s*$/]}"
+        Coradoc::Document::Inline::Quotation.new(content)
+      end
+


Trailing whitespace detected.

hound · 2024-04-19T05:29:36Z

lib/reverse_adoc/converters/sup.rb

-        "#{content[/^\s*/]}^#{content.strip}^#{content[/\s*$/]}"
+        Coradoc::Document::Inline::Superscript.new(content)
+      end
+      def convert(node, state = {})


Use empty lines between method definitions.

ronaldtse · 2024-04-19T06:53:15Z

lib/reverse_adoc/converters/a.rb

+        elsif href.to_s.start_with?('#')
+          href = href.sub(/^#/, "").gsub(/\s/, "").gsub(/__+/, "_")
+          if name.to_s.empty?
+            Coradoc::Document::Inline::CrossReference.new(href)


A cross reference links an optional custom text to a target. The target can be an anchor in the document, or an anchor in a separate document.

Maybe combine this case with the next case into an initialize method that takes parameters.

ronaldtse · 2024-04-19T06:54:32Z

lib/reverse_adoc/converters/aside.rb

-        anchor = id ? "[[#{id}]]\n" : ""
-        content = treat_children(node, state)#.strip
-        Coradoc::Document::Block.new(nil, lines: content.lines, type: :side)
+        content = treat_children(node, state)


Can we call “treat_children” just “parse” on the children?

ronaldtse · 2024-04-19T06:55:27Z

lib/reverse_adoc/converters/blockquote.rb

        content = treat_children(node, state).strip
        content = ReverseAdoc.cleaner.remove_newlines(content)
-        Coradoc::Document::Block.new(nil, lines: content.lines, type: :quote, attributes: attrs)
+        Coradoc::Document::Block::Quote.new(nil, lines: content, attributes: attributes)


What’s the first nil argument? It looks weird.

It's title, previous reverse_adoc code was inconsistent with regards to blocks, I will look into how exactly are different types of asciidoc blocks converted into html

So there are actually 3 types of HTML source:

Normal HTML

Source generated by Metanorma

Source: https://github.com/metanorma/mn-samples-iso

HTML: https://metanorma.github.io/mn-samples-iso/

Source generated by Word

(@opoudjis what are the examples?)

ronaldtse · 2024-04-20T09:32:12Z

@xyz65535 once you're done please help work on this:

Update implementation to be able to convert this HTML file into Adoc #90

Thanks!

hound · 2024-04-25T06:20:17Z

lib/reverse_adoc/converters/video.rb

+        attributes = Coradoc::Document::AttributeList.new
+        options = options(node)
+        attributes.add_named("options", options) if options.any?
+        Coradoc::Document::Video.new(title, id: id, src: src, attributes: attributes)


Line is too long. [85/80]

hound · 2024-04-25T06:20:17Z

spec/components/basic_spec.rb

@@ -32,12 +32,16 @@
  it { is_expected.to match /a \*strong with leading and trailing\* whitespace/ }
  it { is_expected.to match /a \*strong with extra leading and trailing\* whitespace/ }

-  it { is_expected.to match /H~2~O/ }
-  it { is_expected.to match /A\^2\^B/ }
+


Extra blank line detected.

hound · 2024-04-25T06:20:17Z

spec/lib/reverse_adoc/cleaner_spec.rb

+      # it "still cleans up whitespaces that aren't inside a link" do
+      #   input = "now __italic __with following [under__scored](link)"
+      #   result = cleaner.clean_tag_borders(input)
+      #   expect(result).to eq "now __italic__with following [under__scored](link)"


Line is too long. [83/80]

hound · 2024-04-25T06:20:17Z

spec/lib/reverse_adoc/cleaner_spec.rb

+      # it "still cleans up whitespaces that aren't inside a link" do
+      #   input = "now __italic __with following [under__scored](link)"
+      #   result = cleaner.clean_tag_borders(input)
+      #   expect(result).to eq "now __italic__ with following [under__scored](link)"


Line is too long. [84/80]

hound · 2024-04-25T06:20:17Z

spec/lib/reverse_adoc/converters/blockquote_spec.rb

@@ -19,6 +19,6 @@
  it 'can deal with cite attribute' do
    input = node_for("<blockquote cite='http://www.example.com'><p>Some text.</p><p>Some more text.</p></blockquote>")
    result = converter.convert(input)
-    expect(result).to eq "\n\n[quote, http://www.example.com]\n____\nSome text.\n\nSome more text.\n____\n\n"
+    expect(result).to eq "\n\n[quote,http://www.example.com]\n____\nSome text.\n\nSome more text.\n____\n\n"


Line is too long. [108/80]

hound · 2024-04-25T06:20:18Z

lib/reverse_adoc/converters/a.rb

 require "uri"

 module ReverseAdoc
  module Converters
    class A < Base
-      def convert(node, state = {})
+      def to_coradoc(node, state ={})


Assignment Branch Condition size for to_coradoc is too high. [<6, 22, 9> 24.52/15]
Cyclomatic complexity for to_coradoc is too high. [9/6]
Method has too many lines. [17/10]
Perceived complexity for to_coradoc is too high. [10/7]
Surrounding space missing in default value assignment.

hound · 2024-04-25T06:20:18Z

lib/reverse_adoc/converters/aside.rb

-        anchor = id ? "[[#{id}]]\n" : ""
-        content = treat_children(node, state).strip
-        "\n\n****\n" << treat_children(node, state) << "\n****\n\n"
+        Coradoc::Generator.gen_adoc(to_coradoc(node,state))


Space missing after comma.

hound · 2024-04-25T06:20:18Z

lib/reverse_adoc/converters/audio.rb

+        attributes = Coradoc::Document::AttributeList.new
+        options = options(node)
+        attributes.add_named("options", options) if options.any?
+        Coradoc::Document::Audio.new(title, id: id, src: src, attributes: attributes)


Line is too long. [85/80]

hound · 2024-04-25T06:20:18Z

lib/reverse_adoc/converters/audio.rb

-        autoplay   = node['autoplay']
-        loop_attr   = node['loop']
-        controls   = node['controls']
+      def to_coradoc(node, state = {})


Unused method argument - state. If it's necessary, use _ or _state as an argument name to indicate that it won't be used.

hound · 2024-04-25T06:20:18Z

lib/reverse_adoc/converters/base.rb

+
+        before && after
+      end
+


Extra empty line detected at class body end.

hound · 2024-04-26T10:18:58Z

lib/reverse_adoc/converters/img.rb

+        attributes.add_positional(width) if width
+        attributes.add_positional(height) if height
+
+        Coradoc::Document::Image::BlockImage.new(title, id, src, attributes: attributes)


Line is too long. [88/80]

hound · 2024-04-26T10:18:58Z

lib/reverse_adoc/converters/img.rb

+        attributes = Coradoc::Document::AttributeList.new
+        # attributes.add_named("id", id) if id
+        if alt# && !alt.to_s.empty?
+          attributes.add_positional(alt) 


Trailing whitespace detected.

hound · 2024-04-26T10:18:58Z

lib/reverse_adoc/converters/img.rb

-        [anchor, title, "image::", src, "[", attrs, "]"].join("")
+        attributes = Coradoc::Document::AttributeList.new
+        # attributes.add_named("id", id) if id
+        if alt# && !alt.to_s.empty?


Put a space before an end-of-line comment.

hound · 2024-04-26T10:18:58Z

lib/reverse_adoc/converters/img.rb

@@ -55,26 +55,35 @@ def copy_temp_file(imgdata)
        end
      end

-      def convert(node, state = {})
+      def to_coradoc(node, state = {})
+        id = node['id']


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

hound · 2024-04-26T10:18:58Z

lib/reverse_adoc/converters/img.rb

@@ -55,26 +55,35 @@ def copy_temp_file(imgdata)
        end
      end

-      def convert(node, state = {})
+      def to_coradoc(node, state = {})


Assignment Branch Condition size for to_coradoc is too high. [<8, 15, 6> 18.03/15]
Cyclomatic complexity for to_coradoc is too high. [7/6]
Method has too many lines. [21/10]
Perceived complexity for to_coradoc is too high. [8/7]
Unused method argument - state. If it's necessary, use _ or _state as an argument name to indicate that it won't be used.

hound · 2024-04-26T10:18:59Z

lib/reverse_adoc/converters/base.rb

+          before = true
+        end
+        if !before
+          before = true if node.to_s[0] =~ /\s/


Consider merging nested conditions into outer if conditions.
Use match? instead of =~ when MatchData is not used.

hound · 2024-04-26T10:18:59Z

lib/reverse_adoc/converters/base.rb

+      def constrained?(node)
+        before = node.at_xpath("preceding::node()[1]").to_s[-1]
+        if before
+          before = (before =~ (/\s/)) ? true : false


Omit parentheses for ternary conditions.

hound · 2024-04-26T10:18:59Z

lib/reverse_adoc/converters/base.rb

+
+      def constrained?(node)
+        before = node.at_xpath("preceding::node()[1]").to_s[-1]
+        if before


Use the return of the conditional for variable assignment and comparison.

hound · 2024-04-26T10:18:59Z

lib/reverse_adoc/converters/base.rb

+
+      #def trailing_whitespace?(node)
+
+      def constrained?(node)


Assignment Branch Condition size for constrained? is too high. [<8, 16, 11> 21/15]
Cyclomatic complexity for constrained? is too high. [10/6]
Method has too many lines. [19/10]
Perceived complexity for constrained? is too high. [12/7]

hound · 2024-04-26T10:18:59Z

lib/reverse_adoc/converters/base.rb

+        end
+      end
+
+      #def trailing_whitespace?(node)


Missing space after #.

* coradoc integration --------- Co-authored-by: Ronald Tse <[email protected]>

coradoc integration

400f128

hound bot reviewed Apr 17, 2024

View reviewed changes

ronaldtse requested changes Apr 18, 2024

View reviewed changes

webdev778 requested a review from ronaldtse April 19, 2024 01:30

coradoc integration cont.

be0c4f7

hound bot reviewed Apr 19, 2024

View reviewed changes

ronaldtse reviewed Apr 19, 2024

View reviewed changes

coradoc integration cont.

9938958

hound bot reviewed Apr 25, 2024

View reviewed changes

ronaldtse changed the base branch from main to xyz65535-coradoc-integration April 26, 2024 10:16

Merge branch 'xyz65535-coradoc-integration' into coradoc_integration

d3838b7

hound bot reviewed Apr 26, 2024

View reviewed changes

ronaldtse merged commit 42d46b2 into metanorma:xyz65535-coradoc-integration Apr 26, 2024
2 of 6 checks passed

ronaldtse added a commit that referenced this pull request Apr 26, 2024

coradoc integration (#89)

34d00de

* coradoc integration --------- Co-authored-by: Ronald Tse <[email protected]>

coradoc integration #89

coradoc integration #89

Conversation

xyz65535 commented Apr 17, 2024 • edited Loading

Metanorma PR checklist

hound bot Apr 17, 2024

Choose a reason for hiding this comment

hound bot Apr 17, 2024

Choose a reason for hiding this comment

hound bot Apr 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

hound bot Apr 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronaldtse commented Apr 20, 2024

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 25, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

Choose a reason for hiding this comment

hound bot Apr 26, 2024

xyz65535 commented Apr 17, 2024 •

edited

Loading