Document new KVP API in Doxygen, including relevant bits from src/docs/design.

src/engine/kvp_doc.txt

@@ -16,7 +16,7 @@ Name: The name of the key, including key names of parent frames
       filename. Use the '/' character to separate keys.
 
 Type: The type of value stored in the key. The types are listed in
-      'kvp_frame.h'.
+      'kvp-value.hpp'.
 
 Entities: Which engine entities (Accounts, Transactions, Splits)
           can use this key. Use 'All' if every entity could have

src/libqof/qof/kvp-value.hpp

@@ -37,6 +37,12 @@ extern "C"
 
 //Must be a struct because it's exposed to C so that it can in turn be
 //translated to/from Scheme.
+/** @addtogroup KVP
+ * @{
+ */
+
+/** Implements KvpValue using boost::variant.
+ */
 struct KvpValueImpl
 {
     public:
@@ -134,7 +140,7 @@ struct KvpValueImpl
 
 int
 compare(const KvpValueImpl *, const KvpValue *) noexcept;
-
+/** @} Close Doxygen AddToGroup */
 template <typename T>
 KvpValueImpl::KvpValueImpl(T newvalue) noexcept:
     datastore(newvalue)
@@ -153,7 +159,9 @@ KvpValueImpl::set(T val) noexcept
 {
     this->datastore = val;
 }
-
+/** @ingroup KVP
+    @{ */
+/** @internal @{ */
 /** Convert a kvp_value into a GValue. Frames aren't converted.
  * @param kval: A KvpValue.
  * @return GValue*. Must be freed with g_free().
@@ -172,7 +180,8 @@ KvpValue* kvp_value_from_gvalue (const GValue *gval);
  * \param value: A GValue* created by kvp_frame_get_gvalue
  */
 void gnc_gvalue_free (GValue *value);
-
+/** @} Close Doxygen Internal */
+/** @} Close Doxygen Group */
 extern "C" GType gnc_value_list_get_type (void);
 #define GNC_TYPE_VALUE_LIST (gnc_value_list_get_type ())
 

src/libqof/qof/kvp_frame.hpp

@@ -39,6 +39,35 @@
  * passed as either '/'-delimited strings or as std::vectors of keys. Unlike
  * file system paths, the tokens '.' and '..' have no special meaning.
  *
+ * KVP is an implementation detail whose direct use should be avoided; create an
+ * abstraction object in libqof to keep KVP encapsulated here and ensure that
+ * KVP modifications are written to the database. Two generic abstractions are
+ * provided:
+ *
+ * * @ref qof_instance_set_kvp and @ref qof_instance_get_kvp provide single-item
+     access via GValues to support object properties.
+
+ * * @ref qof_book_set_option and @ref qof_book_get_option provide similar
+     access for book options.
+
+ *
+ * @ref kvpvalues provides a catolog of KVP entries including what objects
+ * they're part of and how they're used.
+ *
+ * ## Purpose
+ * KVP is used to extend the class structure without directly reflecting the extension in the database or xML schema. The backend will directly load and store KVP slots without any checking, which allows older versions of GnuCash to load the database without complaint and without damaging the KVP data that they don't understand.
+ *
+ * When a feature is entirely implemented in KVP and doesn't affect the meaning of the books or other features, this isn't a problem, but when it's not true then it should be registered in @ref UtilFeature so that older versions of GnuCash will refuse to load the database.
+ *
+ * ## Policy
+ * * Document every KVP slot in src/engine/kvp_doc.txt so that it is presented
+ * in @ref kvpvalues.
+ * * Register a feature in @ref UtilFeature if the use of the KVP in any way affects the books or business computations.
+ * * Key strings should be all lower case with '-', not spaces, separating words and '/' separating frames. Prefer longer and more descriptive names to abbreviations, and define a global const char[] to the key or path string so that the compiler will catch any typos.
+ * * Make good use of the hierarchical nature of KVP by using frames to group related slots.
+ * * Don't use the KVP API directly outside of libqof, and prefer to use the QofInstance and QofBook functions whenever feasible. If those functions aren't feasible write a class in libqof to abstract the use of KVP.
+ * * Avoid re-using key names in different contexts (e.g. Transactions and Splits) unless the slot is used for the same purpose in both.
+ * @{
 */
 
 #ifndef GNC_KVP_FRAME_TYPE
@@ -49,7 +78,22 @@
 #include <string>
 #include <vector>
 #include <cstring>
+using Path = std::vector<std::string>;
 
+/** Implements KvpFrame.
+ *  It's a struct because QofInstance needs to use the typename to declare a
+ *  KvpFrame* member, and QofInstance's API is C until its children are all
+ *  rewritten in C++.
+ *
+ * N.B.**  Writes to KvpFrames must** be wrapped in BeginEdit and Commit
+ * for the containing QofInstance and the QofInstance must be marked dirty. This
+ * is not** done by the KvpFrame API. In general Kvp items should be
+ * accessed using either QofInstance or QofBook methods in order to ensure that
+ * this is done.
+ * @{
+ */
+struct KvpFrameImpl
+{
     class cstring_comparer
     {
     public:
@@ -60,12 +104,7 @@ class cstring_comparer
 		return ret;
 	    }
     };
-
+    using map_type = std::map<const char *, KvpValue*, cstring_comparer>;
-using Path = std::vector<std::string>;
-
-struct KvpFrameImpl
-{
-    typedef std::map<const char *, KvpValue*, cstring_comparer> map_type;
 
     public:
     KvpFrameImpl() noexcept {};
@@ -98,8 +137,8 @@ struct KvpFrameImpl
      * Set the value with the key in a subframe following the keys in path,
      * replacing and returning the old value if it exists or nullptr if it
      * doesn't. Creates any missing intermediate frames.
-     * @param path: The path of subframes as a '/'-delimited string leading to the frame in which to
+     * @param path: The path of subframes as a '/'-delimited string leading to
-     * insert/replace.
+     * the frame in which to insert/replace.
      * @param newvalue: The value to set at key.
      * @return The old value if there was one or nullptr.
      */
@@ -112,8 +151,8 @@ struct KvpFrameImpl
      * Set the value with the key in a subframe following the keys in path,
      * replacing and returning the old value if it exists or nullptr if it
      * doesn't. Creates any missing intermediate frames.
-     * @param path: The path of subframes as a std::vector leading to the frame in which to
+     * @param path: The path of subframes as a std::vector leading to the
-     * insert/replace.
+     * frame in which to insert/replace.
      * @param newvalue: The value to set at key.
      * @return The old value if there was one or nullptr.
      */
@@ -158,4 +197,6 @@ struct KvpFrameImpl
 
 int compare (const KvpFrameImpl &, const KvpFrameImpl &) noexcept;
 int compare (const KvpFrameImpl *, const KvpFrameImpl *) noexcept;
+/** @} Doxygen Group */
+
 #endif

src/libqof/qof/qofbook.h

@@ -354,6 +354,9 @@ void qof_book_begin_edit(QofBook *book);
 void qof_book_commit_edit(QofBook *book);
 
 /* Access functions for options. */
+/** @ingroup KVP
+ @{
+ */
 /** Load a GNCOptionsDB from KVP data.
  * @param book: The book.
  * @param load_cb: A callback function that does the loading.
@@ -394,7 +397,7 @@ KvpValue* qof_book_get_option (QofBook *book, GSList *path);
  * @param list: A GList of keys which from a path under KVP_OPTION_PATH.
  */
 void qof_book_options_delete (QofBook *book);
-
+/** @} End of Doxygen Include */
 /** deprecated */
 #define qof_book_get_guid(X) qof_entity_get_guid (QOF_INSTANCE(X))
 

src/libqof/qof/qofinstance-p.h

@@ -106,11 +106,33 @@ void qof_instance_set_version_check (gpointer inst, guint32 value);
 void qof_instance_copy_version_check (gpointer to, gconstpointer from);
 void qof_instance_set_idata(gpointer inst, guint32 idata);
 /* Convenience functions to save some typing in property handlers */
+/** @ingroup KVP
+ * @{ */
+/** Report whether a QofInstance has anything stored in KVP
+ * @param inst The QofInstance
+ * @return TRUE if Kvp isn't empty.
+ */
 gboolean qof_instance_has_kvp (QofInstance *inst);
+/** Sets a KVP slot to a value from a GValue. The key can be a '/'-delimited
+ * path, and intermediate container frames will be created if necessary.
+ * Commits the change to the QofInstance.
+ * @param inst: The QofInstance on which to set the value.
+ * @param key: The key for the slot or '/'-delimited path
+ * @param value: A GValue containing an item of a type which KvpValue knows
+ * how to store.
+ */
 void qof_instance_set_kvp (QofInstance *inst, const gchar *key, const GValue *value);
-void qof_instance_get_kvp (const QofInstance *inst, const gchar *key, GValue *value);
+/** Retrieves the contents of a KVP slot into a provided GValue.
-/* Functions to isolate the KVP mechanism inside QOF for cases where GValue
+ * @param inst: The QofInstance
- * operations won't work.
+ * @param key: The key of or '/'-delimited path to the slot.
+ * @param value: A GValue into which to store the value of the slot. It will be
+ *               set to the correct type.
+ */
+void qof_instance_get_kvp (const QofInstance *inst, const gchar *key, GValue
+*value);
+/** @} Close out the DOxygen ingroup */
+/* Functions to isolate the KVP mechanism inside QOF for cases where
+GValue * operations won't work.
  */
 void qof_instance_copy_kvp (QofInstance *to, const QofInstance *from);
 void qof_instance_swap_kvp (QofInstance *a, QofInstance *b);

src/libqof/qof/qofinstance.h

@@ -66,13 +66,7 @@ typedef struct KvpFrameImpl KvpFrame;
 struct QofInstance_s
 {
     GObject object;
-
     QofIdType        e_type;		   /**<	Entity type */
-
-    /* kvp_data is a key-value pair database for storing arbirtary
-     * information associated with this instance.
-     * See src/engine/kvp_doc.txt for a list and description of the
-     * important keys. */
     KvpFrame *kvp_data;
 };