Internal handling of extra_options seems inconsistent

[chrisjsewell]

This issue arises from when I was looking into the code considering #1082 and #1119
(@danwos as we discussed this morning)

The use of the config variable needs_extra_options and of the global variable NEEDS_CONFIG.extra_options seem inconsistent, in that:

1. Sometimes needs_extra_options is used, where it seems that NEEDS_CONFIG.extra_options should probably be used instead
2. NEEDS_CONFIG.extra_options is difficult to track, because at different stages it has new items added, which are obviously not taken into account in previous stages.

Below is a summary of their use in the code base:

---

Extra options are created by either:

1. A user adds them via the needs_extra_options configuration, as a list[str]
2. A user / extension uses api.configuration.add_extra_option to "programmatically" add a name to NEEDS_CONFIG.extra_options (raises NeedsApiConfigWarning if already added). For example sphinx-test-reports uses this in a config-inited event

They are used in the following places (in this order):

• In load_config (config-inited event)

  • needs_extra_options are added to NEEDS_CONFIG.extra_options object (a warning is raised if one already exists)
  • needs_duration_option (default duration) + needs_completion_option (default completion) are also added if not present (as of 👌 Remove hard-coding of completion and duration need fields #1127)
  • each NEEDS_CONFIG.extra_options is added as an option on:
    • NeedDirective
    • NeedserviceDirective
    • NeedextendDirective (and also as +<name> -> unchanged and -<name> -> flag)

• In check_configuration (config-inited event):

  • needs_filter_data keys are checked against needs_extra_options names; matching keys cause a NeedsConfigException
  • needs_extra_options names are checked against INTERNALS list; matching keys cause a NeedsConfigException
  • link type names (and their corresponding <name>_back key) are checked against needs_extra_options names; matching keys cause a NeedsConfigException
  • ensure all needs_variant_options keys match a needs_extra_options name, or a link type, or a default option; if not, a NeedsConfigException is raised

• In prepare_env (env-before-read-docs event):

  1. the ServiceManager is initialised and the GithubService and OpenNeedsService classes are registered via ServiceManager.register. In ServiceManager.register, for all options on the service class, the option is added to NEEDS_CONFIG.extra_options (if not already present)

  2. additional "default" extra names are added to NEEDS_CONFIG.extra_options: hidden (removed in 🔧 Remove hidden field from extra_options #1124), duration, completion (removed 👌 Remove hard-coding of completion and duration need fields #1127), constraints (removed in 🔧 Remove constraints from extra_options #1123) (if not already present)

• In load_external_needs (env-before-read-docs event) for each loaded need, all fields in needs_extra_options are always kept (this data is then parsed on to add_external_need -> add_need)

• In NeedDirective.run (called during read) for all names in NEEDS_CONFIG.extra_options gather pass kwargs to add_need, i.e. kwarks[name] = self.options.get(name, "")

  • It is of note that duration and completion are specifically gathered, but then surely must be overridden, since these are both added as defaults above (in prepare_env)

• In NeedimportDirective.run, NEEDS_CONFIG.extra_options are added to the known_options list then, for each import need item, if a field is not in known_options it is dropped, the rest are passed to add_need

  • This is similar to the logic in load_external_needs, BUT that uses needs_extra_options instead of NEEDS_CONFIG.extra_options

• In add_need (called before/during read) NEEDS_CONFIG.extra_options is passed to _merge_extra_options, as well as the kwargs passed to add_need:

  • All kwargs passed to add_need must either be in NEEDS_CONFIG.extra_options or in needs_extra_links (or a NeedsInvalidOption is raised)

  • In _merge_extra_options, for all the NEEDS_CONFIG.extra_options keys:

    • if the key is in kwargs, but not already set by add_need (i.e. is not an internal key), its value is set to kwargs[key], or
    • if the key is not already set by add_need (i.e. is not an internal key), it is set to a default value of ""

• In NeedReportDirective, if required, need_extra_options are listed

[chrisjsewell]

One thing else I note @danwos:
I see now that the duration and completion fields are added for use by the needgantt directive.

For this there are also needs_duration_option and needs_completion_option configuration options, to change the name of these fiels.
However, since duration and completion are hard-coded in the code base in a number of places,
I think if users were to use these configurations options to change the names, this would not work very well