Skip to content

Rework and improve documentation for "list builders"

eta requested to merge eta/arti:fix-list-builders into main
  • We currently special-case lists of things inside our config builder framework. Arguably this is overcomplicated, but it provides some benefits, like being able to say "give me the default value of this list and append my value on".
  • Currently, the macros used to access these special-cased lists discard documentation, and are somewhat obscure-sounding and hard to understand from an external perspective.
    • In particular, the type of the field is not easy to discern; it's an Option<Vec<T>>, but it's kind of unclear that this is the case.
  • To solve this, we rename the functions, ending up with three functions per list:
    • [field]_mut, which returns &mut Option<Vec<T>>
    • [field]_ref, which returns &Option<Vec<T>>
    • [field]_or_insert_default, which returns &mut Vec<T> after inserting the default value into the field if it is None.
  • This helps expose what the field actually is (an optional vector), while preserving the ability to use the default value, adopting the naming used by similar methods in the standard library (like the Entry API).
  • In addition, specifying documentation in the macro invocation is now mandatory, and this is copied into the doc comments for the generated functions.
    • Note that this still leaves the rest of the builder API surface questionably documented.

Commentary: I found the whole list builder macrology (and indeed, a lot of the builder stuff in general) really rather obscure and hard to follow; it took me a good 30 minutes to get my head around it. While using builders does provide a lot of nice API stability guarantees and other nice stuff, I really can't help but wonder whether there's a simpler approach we've missed; perhaps this is something to discuss as a team at some stage, just to make sure we're not stuck with this for 1.0?

fixes #500; supersedes and closes !595 (closed)

Merge request reports