Apply documentation suggestions from code review

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>

doc/external-api/README.md

@@ -1,11 +1,27 @@
 # Getting started
 
-There are two ways to interface with nix: embedding it, or as a plugin. Embedding means you link one of the nix libraries in your program and use it from there, while being a plugin means you make a library that gets loaded by the nix evaluator, specified through a configuration option.
+These C bindings are **experimental** at the moment, which means they can still change any time or get removed again, but the plan is to provide a stable external C API to the Nix language and the Nix store.
+
+The language library allows evaluating Nix expressions and interacting with Nix language values.
+The Nix store API is still rudimentary, and only allows initialising and connecting to a store for the Nix language evaluator to interact with.
+
+Currently there are two ways to interface with the Nix language evaluator programmatically:
+1. Embedding the evaluator
+2. Writing language plug-ins
+
+Embedding means you link the Nix C libraries in your program and use them from there.
+Adding a plug-in means you make a library that gets loaded by the Nix language evaluator, specified through a configuration option.
+
+Many of the components and mechanisms involved are not yet documented, therefore please refer to the [Nix source code](https://github.com/NixOS/nix/) for details.
+Additions to in-code documentation and the reference manual are highly appreciated.
+
+
+The following examples, for simplicity, don't include error handling.
+See the [Handling errors](@ref errors) section for more information.
 
 # Embedding the Nix Evaluator
 
-These examples don't include error handling.
+In this example we programmatically start the Nix language evaluator with a dummy store (that has no store paths and cannot be written to), and evaluate the Nix expression `builtins.nixVersion`.
-See the [Handling errors](@ref errors) section for more information.
 
 **main.c:**
 ```C
@@ -18,7 +34,7 @@ int main() {
    nix_libexpr_init(NULL);
 
    Store* store = nix_store_open(NULL, "dummy://", NULL);
-   State* state = nix_state_create(NULL, NULL, store); // empty nix path
+   State* state = nix_state_create(NULL, NULL, store); // empty search path (NIX_PATH)
    Value *value = nix_alloc_value(NULL, state);
 
    nix_expr_eval_from_string(NULL, state, "builtins.nixVersion", ".", value);
@@ -40,7 +56,9 @@ nix version 1.2.3
 ```
 
 
-# Writing a Nix Plugin
+# Writing a Nix language plug-in
+In this example we add a custom primitive operation (*primop*) to `builtins`.
+It will increment the argument if it is an integer and return `null` otherwise.
 
 **plugin.c:**
 ```C
@@ -59,7 +77,7 @@ void increment(State* state, int pos, Value** args, Value* v) {
  
 void nix_plugin_entry() {
   const char* args[] = {"n", NULL};
-  PrimOp *p = nix_alloc_primop(NULL, increment, 1, "increment", args, "Example nix plugin function: increments an int");
+  PrimOp *p = nix_alloc_primop(NULL, increment, 1, "increment", args, "Example custom built-in function: increments an integer");
   nix_register_primop(NULL, p);
   nix_gc_decref(NULL, p);
 }

doc/external-api/doxygen.cfg.in

@@ -18,7 +18,7 @@ PROJECT_NUMBER         = @PACKAGE_VERSION@
 # for a project that appears at the top of each page and should give viewer a
 # quick idea about the purpose of the project. Keep the description short.
 
-PROJECT_BRIEF          = "Nix, the purely functional package manager; stable external interfaces"
+PROJECT_BRIEF          = "Nix, the purely functional package manager: C API (experimental)"
 
 # If the GENERATE_LATEX tag is set to YES, doxygen will generate LaTeX output.
 # The default value is: YES.

src/libexpr/c/nix-expr-c.pc.in

@@ -3,7 +3,7 @@ libdir=@libdir@
 includedir=@includedir@
 
 Name: Nix
-Description: Nix Package Manager - C API
+Description: Nix Language Evaluator - C API
 Version: @PACKAGE_VERSION@
 Requires: nix-store-c
 Libs: -L${libdir} -lnixexprc

src/libexpr/c/nix_api_expr.h

@@ -1,7 +1,7 @@
 #ifndef NIX_API_EXPR_H
 #define NIX_API_EXPR_H
 /** @defgroup libexpr libexpr
- * @brief Bindings to the Nix evaluator
+ * @brief Bindings to the Nix language evaluator
  *
  * Example (without error handling):
  * @code{.c}
@@ -38,17 +38,18 @@ extern "C" {
 
 // Type definitions
 /**
- * @brief Represents a nix evaluator state.
+ * @brief Represents a state of the Nix language evaluator.
  *
- * Multiple can be created for multi-threaded
+ * Multiple states can be created for multi-threaded
  * operation.
  * @struct State
+ * @see nix_state_create
  */
 typedef struct State State; // nix::EvalState
 /**
- * @brief Represents a nix value.
+ * @brief Represents a value in the Nix language.
  *
- * Owned by the GC.
+ * Owned by the garbage collector.
  * @struct Value
  * @see value_manip
  */
@@ -56,7 +57,7 @@ typedef void Value; // nix::Value
 
 // Function prototypes
 /**
- * @brief Initializes the Nix expression evaluator.
+ * @brief Initialize the Nix language evaluator.
  *
  * This function should be called before creating a State.
  * This function can be called multiple times.
@@ -73,6 +74,7 @@ nix_err nix_libexpr_init(nix_c_context *context);
  * @param[in] state The state of the evaluation.
  * @param[in] expr The Nix expression to parse.
  * @param[in] path The file path to associate with the expression.
+ * This is required for expressions that contain relative paths (such as `./.`) that are resolved relative to the given directory.
  * @param[out] value The result of the evaluation. You should allocate this
  * yourself.
  * @return NIX_OK if the evaluation was successful, an error code otherwise.
@@ -109,7 +111,7 @@ nix_err nix_value_call(nix_c_context *context, State *state, Value *fn,
  * @param[out] context Optional, stores error information
  * @param[in] state The state of the evaluation.
  * @param[in,out] value The Nix value to force.
- * @post values is not of type NIX_TYPE_THUNK
+ * @post value is not of type NIX_TYPE_THUNK
  * @return NIX_OK if the force operation was successful, an error code
  * otherwise.
  */
@@ -133,10 +135,10 @@ nix_err nix_value_force_deep(nix_c_context *context, State *state,
                              Value *value);
 
 /**
- * @brief Creates a new Nix state.
+ * @brief Create a new Nix language evaluator state.
  *
  * @param[out] context Optional, stores error information
- * @param[in] searchPath The NIX_PATH.
+ * @param[in] searchPath Array of strings corresponding to entries in NIX_PATH.
  * @param[in] store The Nix store to use.
  * @return A new Nix state or NULL on failure.
  */
@@ -155,28 +157,28 @@ void nix_state_free(State *state);
 /** @addtogroup GC
  * @brief Reference counting and garbage collector operations
  *
- * Nix's evaluator uses a garbage collector. To ease C interop, we implement
+ * The Nix language evaluator uses a garbage collector. To ease C interop, we implement
  * a reference counting scheme, where objects will be deallocated
  * when there are no references from the Nix side, and the reference count kept
  * by the C API reaches `0`.
  *
  * Functions returning a garbage-collected object will automatically increase
  * the refcount for you. You should make sure to call `nix_gc_decref` when
- * you're done.
+ * you're done with a value returned by the evaluator.
  * @{
  */
 /**
- * @brief Increase the GC refcount.
+ * @brief Increment the garbage collector reference counter for the given object.
  *
- * The nix C api keeps alive objects by refcounting.
+ * The Nix language evaluator C API keeps track of alive objects by reference counting.
- * When you're done with a refcounted pointer, call nix_gc_decref.
+ * When you're done with a refcounted pointer, call nix_gc_decref().
  *
  * @param[out] context Optional, stores error information
  * @param[in] object The object to keep alive
  */
 nix_err nix_gc_incref(nix_c_context *context, const void *object);
 /**
- * @brief Decrease the GC refcount
+ * @brief Decrement the garbage collector reference counter for the given object
  *
  * @param[out] context Optional, stores error information
  * @param[in] object The object to stop referencing
@@ -193,7 +195,7 @@ void nix_gc_now();
 /**
  * @brief Register a callback that gets called when the object is garbage
  * collected.
- * @note objects can only have a single finalizer. This function overwrites
+ * @note Objects can only have a single finalizer. This function overwrites existing values
  * silently.
  * @param[in] obj the object to watch
  * @param[in] cd the data to pass to the finalizer

src/libexpr/c/nix_api_external.h

@@ -22,7 +22,7 @@ extern "C" {
 // cffi start
 
 /**
- * @brief Represents a string owned by nix.
+ * @brief Represents a string owned by the Nix language evaluator.
  * @see nix_set_owned_string
  */
 typedef struct nix_string_return nix_string_return;

src/libexpr/c/nix_api_value.h

@@ -73,9 +73,10 @@ typedef struct ExternalValue ExternalValue;
  */
 typedef void (*PrimOpFun)(State *state, int pos, Value **args, Value *v);
 
-/** @brief Allocate a primop
+/** @brief Allocate a PrimOp
  *
- * Owned by the GC. Use nix_gc_decref when you're done with the pointer
+ * Owned by the garbage collector.
+ * Use nix_gc_decref() when you're done with the returned PrimOp.
  *
  * @param[out] context Optional, stores error information
  * @param[in] fun callback
@@ -89,12 +90,12 @@ typedef void (*PrimOpFun)(State *state, int pos, Value **args, Value *v);
 PrimOp *nix_alloc_primop(nix_c_context *context, PrimOpFun fun, int arity,
                          const char *name, const char **args, const char *doc);
 
-/** @brief add a primop to builtins
+/** @brief add a primop to the `builtins` attribute set
  *
- * Only applies to new States.
+ * Only applies to States created after this call.
  *
- * Moves your primop into the global
+ * Moves your PrimOp into the global evaluator
- * registry, meaning your input primOp is no longer usable
+ * registry, meaning your input PrimOp pointer is no longer usable
  * (but still possibly subject to garbage collection).
  *
  * @param[out] context Optional, stores error information
@@ -108,7 +109,7 @@ nix_err nix_register_primop(nix_c_context *context, PrimOp *primOp);
 
 /** @brief Allocate a Nix value
  *
- * Owned by the GC. Use nix_gc_decref when you're done with the pointer
+ * Owned by the GC. Use nix_gc_decref() when you're done with the pointer
  * @param[out] context Optional, stores error information
  * @param[in] state nix evaluator state
  * @return value, or null in case of errors
@@ -116,7 +117,7 @@ nix_err nix_register_primop(nix_c_context *context, PrimOp *primOp);
  */
 Value *nix_alloc_value(nix_c_context *context, State *state);
 /** @addtogroup value_manip Manipulating values
- * @brief Functions to inspect and change nix Value's
+ * @brief Functions to inspect and change Nix language values, represented by Value.
  * @{
  */
 /** @name Getters
@@ -128,7 +129,7 @@ Value *nix_alloc_value(nix_c_context *context, State *state);
  * @return type of nix value
  */
 ValueType nix_get_type(nix_c_context *context, const Value *value);
-/** @brief Get type name of value
+/** @brief Get type name of value as defined in the evaluator
  * @param[out] context Optional, stores error information
  * @param[in] value Nix value to inspect
  * @return type name, owned string

src/libstore/c/nix-store-c.pc.in

@@ -3,7 +3,7 @@ libdir=@libdir@
 includedir=@includedir@
 
 Name: Nix
-Description: Nix Package Manager - C API
+Description: Nix Store - C API
 Version: @PACKAGE_VERSION@
 Libs: -L${libdir} -lnixstorec -lnixutilc
 Cflags: -I${includedir}/nix

src/libstore/c/nix_api_store.h

@@ -19,9 +19,9 @@ extern "C" {
 #endif
 // cffi start
 
-/** @brief reference to a nix store */
+/** @brief Reference to a Nix store */
 typedef struct Store Store;
-/** @brief nix store path */
+/** @brief Nix store path */
 typedef struct StorePath StorePath;
 
 /**
@@ -79,9 +79,9 @@ nix_err nix_store_get_uri(nix_c_context *context, Store *store, char *dest,
 
 // returns: owned StorePath*
 /**
- * @brief parse a nix store path into a StorePath
+ * @brief Parse a Nix store path into a StorePath
  *
- * Don't forget to free this path using nix_store_path_free
+ * @note Don't forget to free this path using nix_store_path_free()!
  * @param[out] context Optional, stores error information
  * @param[in] store nix store reference
  * @param[in] path Path string to parse, copied
@@ -90,7 +90,7 @@ nix_err nix_store_get_uri(nix_c_context *context, Store *store, char *dest,
 StorePath *nix_store_parse_path(nix_c_context *context, Store *store,
                                 const char *path);
 
-/** @brief Deallocate a nix StorePath
+/** @brief Deallocate a StorePath
  *
  * Does not fail.
  * @param[in] p the path to free
@@ -98,9 +98,9 @@ StorePath *nix_store_parse_path(nix_c_context *context, Store *store,
 void nix_store_path_free(StorePath *p);
 
 /**
- * @brief check if a storepath is valid (exists in the store)
+ * @brief Check if a StorePath is valid (i.e. that corresponding store object and its closure of references exists in the store)
  * @param[out] context Optional, stores error information
- * @param[in] store nix store reference
+ * @param[in] store Nix Store reference
  * @param[in] path Path to check
  * @return true or false, error info in context
  */
@@ -109,12 +109,12 @@ bool nix_store_is_valid_path(nix_c_context *context, Store *store,
 // nix_err nix_store_ensure(Store*, const char*);
 // nix_err nix_store_build_paths(Store*);
 /**
- * @brief Build a nix store path
+ * @brief Realise a Nix store path
  *
  * Blocking, calls cb once for each built output
  *
  * @param[out] context Optional, stores error information
- * @param[in] store nix store reference
+ * @param[in] store Nix Store reference
  * @param[in] path Path to build
  * @param[in] userdata data to pass to every callback invocation
  * @param[in] cb called for every built output