Merge pull request #18 from cxw42/issue15

Text justification; font name; space between paragraphs

Author: cxw42

Makefile.am

@@ -37,3 +37,8 @@ prep:
 	+$(MAKE) check
 	+$(MAKE) all build-tests && prove -v
 	+$(MAKE) distcheck
+
+# Used by coverage.sh --- remove the existing code-coverage data.
+remove-code-coverage-data:
+	-rm -rf "$(CODE_COVERAGE_OUTPUT_FILE)" "$(CODE_COVERAGE_OUTPUT_FILE).tmp" "$(CODE_COVERAGE_OUTPUT_DIRECTORY)"
+	-find . -name "*.gcda" -delete

configure.ac

@@ -1,5 +1,5 @@
 dnl === Basic setup =======================================================
-AC_INIT([pfft markdown-to-PDF converter], [0.0.3], [], [pfft], [https://github.com/cxw42/pfft])
+AC_INIT([pfft markdown-to-PDF converter], [0.0.4], [], [pfft], [https://github.com/cxw42/pfft])
 AC_PREREQ([2.65])
 AC_COPYRIGHT([Copyright (C) 2020 Christopher White])
 AC_CONFIG_SRCDIR([rules.mk])    dnl make sure the srcdir is correctly specified

coverage.sh

@@ -1,6 +1,18 @@
 #!/bin/bash
 set -x
 set -eEuo pipefail
-./bootstrap
-./configure --enable-code-coverage USER_VALAFLAGS='-g' CFLAGS='-g -O0' "$@"
+
+# Reconfigure if necessary so that coverage is enabled
+if [[ ! -x ./config.status ]] || \
+        ! ./config.status --config | grep -- '--enable-code-coverage'
+then
+    ./bootstrap
+    ./configure --enable-code-coverage USER_VALAFLAGS='-g' CFLAGS='-g -O0' "$@"
+fi
+
+# Clean up from old runs, e.g., test runs of ./src/pfft compiled with
+# coverage turned on.
+make -j4 remove-code-coverage-data
+
+# Check it
 make -j4 check-code-coverage

rules.mk

@@ -59,6 +59,7 @@ MY_vala_TESTS = \
 	001-sanity-t \
 	010-core-util-t \
 	020-registry-t \
+	021-registry-classmap-t \
 	050-core-el-t \
 	055-core-units-t \
 	060-core-template-t \

src/app/pfft.vala

@@ -8,18 +8,6 @@ namespace My {
 
     extern void init_gstreamer();   // from pfft-shim.c
 
-    // Types {{{1
-
-    /**
-     * Map from friendly names to GTypes.
-     *
-     * Used to store and sort readers and writers.
-     * This is a typedef.
-     */
-    private class ClassMap : Gee.TreeMap<string, GLib.Type> { }
-
-    // }}}1
-
     /**
      * Main application class for pfft
      */
@@ -134,7 +122,7 @@ namespace My {
         {
             Intl.setlocale (LocaleCategory.ALL, "");    // init locale from environment
             init_gstreamer();
-            linit();
+            Log.linit();
         }
 
         /**
@@ -212,8 +200,8 @@ namespace My {
             Reader reader;
             var reader_name = opt_reader_name ?? reader_default_;
             try {
-                reader = create_instance(
-                    readers_, reader_name, opt_reader_options) as Reader;
+                reader = readers_.create_instance(
+                    reader_name, template_, opt_reader_options) as Reader;
             } catch(KeyFileError e) {
                 printerr ("Could not create reader: %s\n", e.message);
                 return 1;
@@ -222,8 +210,8 @@ namespace My {
             Writer writer;
             var writer_name = opt_writer_name ?? writer_default_;
             try {
-                writer = create_instance(
-                    writers_, writer_name, opt_writer_options) as Writer;
+                writer = writers_.create_instance(
+                    writer_name, template_, opt_writer_options) as Writer;
             } catch(KeyFileError e) {
                 printerr ("Could not create writer: %s\n", e.message);
                 return 1;
@@ -417,92 +405,6 @@ namespace My {
             return sb.str;
         } // get_classmap_help
 
-        /**
-         * Create an instance and set its properties.
-         *
-         * Sets properties from template_ first, then from @options.
-         */
-        private Object create_instance(ClassMap m, string class_name,
-            string[]? options) throws KeyFileError
-        {
-            if(!m.has_key(class_name)) {
-                throw new KeyFileError.KEY_NOT_FOUND(
-                          "%s: Class not registered".printf(class_name));
-            }
-
-            var type = m.get(class_name);
-            Object retval = Object.new(type);
-            set_props_from_template(type, retval, template_);
-
-            if(options == null) {
-                return retval;  // *** EXIT POINT ***
-            }
-
-            // property accessor for the instance we are creating
-            ObjectClass ocl = (ObjectClass) type.class_ref ();
-
-            // Assign the properties
-            var num_opts = (options == null) ? 0 : strv_length(options);
-            for(int i=0; i<num_opts; ++i) {
-                var optspec = options[i];
-                var nv = optspec.split("=", 2);
-                if(nv.length != 2) {
-                    throw new KeyFileError.INVALID_VALUE(
-                              "%s: Invalid option %s".printf(class_name, optspec));
-                }
-
-                // print("Trying %p->%s := %s\n", retval, nv[0], nv[1]);
-                var prop = ocl.find_property(nv[0]);
-                if(prop == null || prop.get_name()[0] == 'P') { // skip unknown, private
-                    throw new KeyFileError.KEY_NOT_FOUND(
-                              "%s: %s is not an option I understand".printf(
-                                  class_name, nv[0]));
-                }
-
-                var val = GLib.Value(prop.value_type);
-                if(!deserialize_value(ref val, nv[1])) {
-                    throw new KeyFileError.INVALID_VALUE(
-                              "%s: Invalid value %s for option %s".printf(
-                                  class_name, nv[1], nv[0]));
-                }
-
-                retval.set_property(nv[0], val);
-                linfoo(retval, "Set property %s from command line to %s",
-                    nv[0], val.type() == typeof(string) ? @"'$(val.get_string())'" :
-                    Gst.Value.serialize(val));
-
-            } // foreach option
-
-            return retval;
-        } // create_instance()
-
-        private void set_props_from_template(GLib.Type instance_type,
-            Object instance, Template tmpl)
-        {
-            // property accessor for the instance we are creating
-            ObjectClass ocl = (ObjectClass) instance_type.class_ref ();
-
-            // property accessor for the template
-            ObjectClass tocl = (ObjectClass) tmpl.get_type().class_ref ();
-
-            // Set properties from the template
-            foreach(var tprop in tocl.list_properties()) {
-                string propname = tprop.get_name();
-                ldebugo(instance, "Trying template property %s", propname);
-                var prop = ocl.find_property(propname);
-                if(prop == null || propname[0] == 'P' || prop.value_type != tprop.value_type) {
-                    ldebugo(instance, "--- skipping");
-                    continue;
-                }
-
-                Value v = Value(prop.value_type);
-                tmpl.get_property(propname, ref v);
-                instance.set_property(propname, v);
-                linfoo(instance, "Set property %s from template to %s",
-                    propname, Gst.Value.serialize(v));
-            }
-        } // set_props_from_template()
-
         // }}}1
     } // class App
 } // My

src/core/registry.vala

@@ -8,7 +8,103 @@
 // Copyright (c) 2020 Christopher White.  All rights reserved.
 // SPDX-License-Identifier: BSD-3-Clause
 
+using My.Log;
+
 namespace My {
+
+    /**
+     * Map from friendly names to GTypes.
+     *
+     * Used to store and sort readers and writers.
+     */
+    public class ClassMap : Gee.TreeMap<string, GLib.Type> {
+
+        /** Set object properties from 'name=value' pairs */
+        private static void set_options_on(Object target, string[] options,
+            string class_name)
+        throws KeyFileError
+        {
+            // property accessor for the instance we are creating
+            ObjectClass ocl = (ObjectClass) target.get_type().class_ref ();
+
+            // Assign the properties
+            var num_opts = (options == null) ? 0 : strv_length(options);
+            for(int i=0; i<num_opts; ++i) {
+                var optspec = options[i];
+                var nv = optspec.split("=", 2);
+                if(nv.length != 2 || nv[0].length < 1 || nv[1].length < 1) {
+                    throw new KeyFileError.INVALID_VALUE(
+                              "%s: Invalid option %s".printf(class_name, optspec));
+                }
+
+                var prop = ocl.find_property(nv[0]);
+                if(prop == null || prop.get_name()[0] == 'P') { // skip unknown, private
+                    throw new KeyFileError.KEY_NOT_FOUND(
+                              "%s: %s is not an option I understand".printf(
+                                  class_name, nv[0]));
+                }
+
+                var val = GLib.Value(prop.value_type);
+                if(!deserialize_value(ref val, nv[1])) {
+                    throw new KeyFileError.INVALID_VALUE(
+                              "%s: Invalid value %s for option %s".printf(
+                                  class_name, nv[1], nv[0]));
+                }
+
+                // TODO unit conversion for properties with dimen values, a la
+                // Template.
+
+                target.set_property(nv[0], val);
+                linfoo(target, "Set property %s from command line to %s",
+                    nv[0], val.type() == typeof(string) ? @"'$(val.get_string())'" :
+                    Gst.Value.serialize(val)    // LCOV_EXCL_LINE - can't guarantee this will fire during tests
+                );
+
+            } // foreach option
+        } // set_options_on()
+
+        /**
+         * Create an instance and set its properties.
+         *
+         * Sets properties from @template first, then from @options.
+         * @param m             The class registry to use
+         * @param class_name    The name of the class to instantiate.
+         *                      This may be different from the name in the
+         *                      source code.
+         * @param template      Optional My.Template.  Properties that exist
+         *                      both in the template and the target class
+         *                      will be copied from the target to the
+         *                      new instance.
+         * @param options       Optional 'property=value' assigments.
+         * @return The new instance, or null.
+         */
+        public Object? create_instance(string class_name,
+            Template? template, string[]? options) throws KeyFileError
+        {
+            if(!this.has_key(class_name)) {
+                throw new KeyFileError.KEY_NOT_FOUND(
+                          "%s: Class not registered".printf(class_name));
+            }
+
+            var type = this.get(class_name);
+            Object retval = Object.new(type);
+
+            if(retval == null) {
+                return null;    // LCOV_EXCL_LINE - I don't know any way to force this to happen during testing
+            }
+
+            if(template!=null) {
+                template.set_props_on(retval);
+            }
+
+            if(options != null) {
+                set_options_on(retval, options, class_name);
+            }
+
+            return retval;
+        } // create_instance()
+    } // class ClassMap
+
     /**
      * Get a reference to the registry of classes.
      *
@@ -50,4 +146,5 @@ namespace My {
      * Out of all classes that implement a particular interface, that is.
      */
     public const string CLASS_META_NICK_DEFAULT = "default";
+
 } // My

src/core/template.vala

@@ -66,6 +66,8 @@ namespace My {
         public string footerr { get; set; default = ""; }
 
         // Font parameters
+        [Description(nick = "Font name", blurb = "Font of body text")]
+        public string fontname { get; set; default = "Serif"; }
         [Description(nick = "Font size (pt.)", blurb = "Size of body text, in points (72/in.)")]
         public double fontsizeT { get; set; default = 12; }
 
@@ -316,6 +318,7 @@ namespace My {
             } // footer
 
             if(data.has_group("font")) {
+                set_from_file("fontname", "font", "name");
                 set_from_file("fontsizeT", "font", "size");
                 ldebugo(this, "font size %f pt", fontsizeT);
             }
@@ -328,6 +331,43 @@ namespace My {
             ldebugo(this, "Done processing template file %s", filename);
         } // Template.from_file()
 
+        // --- Using templates -------------------------
+
+        /**
+         * Copy property values into an object.
+         *
+         * For each non-private property that exists both in this template and
+         * the target instance, the value will be copied from the target to the
+         * new instance.
+         *
+         * @param target        The instance to update.  Modified in place.
+         */
+        public void set_props_on(Object target)
+        {
+            // property accessor for the instance we are creating
+            ObjectClass ocl = (ObjectClass) target.get_type().class_ref ();
+
+            // property accessor for the template
+            ObjectClass tocl = (ObjectClass) this.get_type().class_ref ();
+
+            // Set properties from the template
+            foreach(var tprop in tocl.list_properties()) {
+                string propname = tprop.get_name();
+                ldebugo(target, "Trying template property %s", propname);
+                var prop = ocl.find_property(propname);
+                if(prop == null || propname[0] == 'P' || prop.value_type != tprop.value_type) {
+                    ldebugo(target, "  --- skipping");
+                    continue;
+                }
+
+                Value v = Value(prop.value_type);
+                this.get_property(propname, ref v);
+                target.set_property(propname, v);
+                linfoo(target, "Set property %s from template to %s",
+                    propname, Gst.Value.serialize(v));
+            }
+        } // set_props_on()
+
     } // class Template
 
-}
+} // namespace My

src/logging/logging-c.c

@@ -11,6 +11,7 @@ GST_DEBUG_CATEGORY(my_log_category);
 void my_log_linit()
 {
     GST_DEBUG_CATEGORY_INIT(my_log_category, "pfft", 0, "");
+    GST_INFO("pfft logging initialized");
 }
 
 // the following is copied from