Refactoring html_doc_path for inheritance

[ronaldtse]

The method html_path_acme was used in the metanorma-acme gem to avoid inheritance problems of the styling paths. In the isodoc gem, it is called html_doc_path.

However, due to the way this super works (https://github.com/riboseinc/isodoc/blob/601c1870fea386cb54fd8b2f112f680f71bd58a3/lib/isodoc/iso/html_convert.rb#L31), the html_doc_path method doesn't work with inheritance, because the method looks for the template path within the gem itself, and when inherited, it first looks for the base template in the inherited gem, and fails.

In particular, in the asciidoctor-rsd gem, this code fails like this:

      def initialize(options)
        super
        @htmlstylesheet = generate_css(rsd_html_path("htmlstyle.scss"), true, default_fonts(options)) # <<=== PROBLEM
        @htmlcoverpage = rsd_html_path("html_rsd_titlepage.html")
        @htmlintropage = rsd_html_path("html_rsd_intro.html")
        @scripts = rsd_html_path("scripts.html") 
        system "cp #{html_doc_path('logo.svg')} logo.svg"
        @files_to_delete << "logo.svg" # <<=== PROBLEM
      end

This code does an additional two things -- generation of SCSS into CSS, and then adds a file to @files_to_delete. Both of these are problematic:

1. The generation of SCSS should not occur until actual initialization (i.e. we've finally decided which templates to use according to the inheritance structure), not before initialization.
2. The CSS should only be decided to be deleted during initialization (i.e. after the first).

Ideally, all paths (and both 1/2) should be given as options to the initialization method, and only after everything is resolved, the SCSS is generated, then deleted after usage.

[opoudjis]

Currently, all gems provide their own local override html_doc_path, precisely because of this issue. I don't see the difference between providing an override html_doc_path and providing File.dirname(FILE) as an initialisation parameter: you still need to remember to update something in your instantiation.

So currently RSD works with:

      def html_doc_path(file)
        File.join(File.dirname(__FILE__), File.join("html", file))
      end

      def initialize(options)
        super
        @htmlstylesheet = generate_css(html_doc_path("htmlstyle.scss"), true, default_fonts(options))
        @htmlcoverpage = html_doc_path("html_rsd_titlepage.html")
        @htmlintropage = html_doc_path("html_rsd_intro.html")
        @scripts = html_doc_path("scripts.html")
        system "cp #{html_doc_path('logo.jpg')}  logo.jpg"
        @files_to_delete << "logo.jpg"
      end

You'd want this changed to:

isodoc gem:

      def html_doc_path(file)
        File.join(@libdir, File.join("html", file))
      end

rsd gem:

      def initialize(options)
        super
        @libdir = File.dirname(__FILE__)
        @htmlstylesheet = generate_css(html_doc_path("htmlstyle.scss"), true, default_fonts(options))
        @htmlcoverpage = html_doc_path("html_rsd_titlepage.html")
        @htmlintropage = html_doc_path("html_rsd_intro.html")
        @scripts = html_doc_path("scripts.html")
        system "cp #{html_doc_path('logo.jpg')}  logo.jpg"
        @files_to_delete << "logo.jpg"
      end

I can do it, but I don't see why it's worth it.

I do not understand what you mean by (1) at all. The SCSS processing is happening in the initialisation routine for the RSD class, and we have decided what templates we are using (the RSD templates), because we know we are in the RSD class. Where do you want to move that code to, and why?

As for (2), the decision to delete the temporary CSS stylesheet in the working directory is made inside the generate_css call, after the CSS is generated. So it is subsequent to (1).

(Ignore the deletion of the logo image in the foregoing; that is an artefact of the gem using a logo, but it is a separate issue.)

I don't understand what the problem is, and what you want me to do about it.

[ronaldtse]

The issue is we want a single method for loading the custom CSS/templates per standard type; rather than having different methods in different gems for the same thing.

In the super method, instead of “super” then setting the paths, I want to pass the path arguments to super directly.

[opoudjis]

I still don't understand. Are you saying:

IsoDoc::HtmlConvert

def initialize(options)
        ...
        init_css(options)
end

def init_css(options)
        @htmlstylesheet = generate_css(options[:htmlstylesheet], true, default_fonts(options))
        @htmlcoverpage = options[:htmlcoverpage]
        @htmlintropage = options[:htmlintropage]
        @scripts = options[: htmlintropage]
end

IsoDoc::Rsd::HtmlConvert

class IsoDoc::Rsd::HtmlConvert << IsoDoc::HtmlConvert

def initialize(options)
        super
        system "cp #{html_doc_path('logo.jpg')}  logo.jpg"
        @files_to_delete << "logo.jpg"
end

Asciidoctor::Rsd::Convert

      def html_converter(node)
        IsoDoc::Rsd::HtmlConvert.new(
          script: node.attr("script"),
          bodyfont: node.attr("body-font"),
          headerfont: node.attr("header-font"),
          monospacefont: node.attr("monospace-font"),
          titlefont: node.attr("title-font"),
          i18nyaml: node.attr("i18nyaml"),
          scope: node.attr("scope"),
          htmlstylesheet: File.join(File.dirname(__FILE__), File.join("../isodoc/html", "htmlstyle.scss")),
          htmlcoverpage: File.join(File.dirname(__FILE__), File.join("../isodoc/html", "html_rsd_titlepage.html")),
          htmlintroppage: File.join(File.dirname(__FILE__), File.join("../isodoc/html", "html_rsd_intro.html")),
          scripts: File.join(File.dirname(__FILE__), File.join("../isodoc/html", "scripts.html")),
        )
      end

True, we are now parameterising the templates in metanorma-acme. I don't see that this is that much nicer, particularly as the gem-specific template files live inside the gem: the metanorma script doesn't have access to them, and would instead have to use something like this:

IsoDoc::Rsd::HtmlConvert

def scriptLocations
{
          htmlstylesheet: File.join(File.dirname(__FILE__), File.join("html", "htmlstyle.scss")),
          htmlcoverpage: File.join(File.dirname(__FILE__), File.join("html", "html_rsd_titlepage.html")),
          htmlintroppage: File.join(File.dirname(__FILE__), File.join("html", "html_rsd_intro.html")),
          scripts: File.join(File.dirname(__FILE__), File.join("html", "scripts.html")),
}
end

Metanorma::Rsd::Processor

def extract_options(file)
    super.merge(scriptLocations)
end

... I can do this, I just don't see that it's that much better. Is this what you wanted?

[opoudjis]

Passing the template locations in as an argument has the disadvantage that we're permitting consumers to put in their own templates instead, which is a branding disaster.

[ronaldtse]

Passing the template locations in as an argument has the disadvantage that we're permitting consumers to put in their own templates instead, which is a branding disaster.

This sounds exactly like a feature that will be positively received 😉 Not everyone will have the resources to customize their own corporate gem...

[opoudjis]

My concern is extending this facility to all gems. It is already in place in the Acme gem, which we intend as the basis for customisation. But we can always introduce override to disable it in particular gems if needed.

[opoudjis]

I will extend this to metanorma-standoc (metanorma/metanorma-iso#62), whence it will be extended to all inheriting gems; this is now necessary for metanorma-standoc spec, because it has no embedded stylesheets.

[opoudjis]

Put @libdir in all gems, no longer localising html_doc_path() method

[opoudjis]

Resolved this by creating default_fonts and default_file_locations methods in the isodoc subclasses. The isodoc initialisation will look for font values and file locations in the options passed to it; if it does not find them, it will invoke these methods for those values instead.

CSS is now being parsed within isodoc; the @libdir value needs to be initialised now before super.

Implemented in asciidoctor-rsd; will be rolling out to all the other gems.

[opoudjis]

And this is what it looks like:

      def initialize(options)
        @libdir = File.dirname(__FILE__)
        super
        system "cp #{html_doc_path('logo.svg')} logo.svg"
        @files_to_delete << "logo.svg"
      end

      def default_fonts(options)
        {
          bodyfont: (options[:script] == "Hans" ? '"SimSun",serif' : '"Overpass",sans-serif'),
          headerfont: (options[:script] == "Hans" ? '"SimHei",sans-serif' : '"Overpass",sans-serif'),
          monospacefont: '"Space Mono",monospace'
        }
      end

      def default_file_locations
        {
          htmlstylesheet: html_doc_path("htmlstyle.scss"),
          htmlcoverpage: html_doc_path("html_rsd_titlepage.html"),
          htmlintropage: html_doc_path("html_rsd_intro.html"),
          scripts: html_doc_path("scripts.html"),
          wordstylesheet: html_doc_path("wordstyle.scss"),
          standardstylesheet: html_doc_path("rsd.scss"),
          header: html_doc_path("header.html"),
          wordcoverpage: html_doc_path("word_rsd_titlepage.html"),
          wordintropage: html_doc_path("word_rsd_intro.html"),
        }
      end

[ronaldtse]

@opoudjis can we use Ruby native methods like File/Dir to deal with file system usage rather than system? We will need to deal with Windows compatibility one day 😉

[opoudjis]

I'm not at all convinced Windows compatibility is going to go well for us with Ruby. But ok, let me see if I can.

[opoudjis]

Done with all gems using system cp

[opoudjis]

Refactored all gems accordingly, and publishing them.