Add needed import statement to needs_extra_options #67

Activity

1. danwos

   added

   enhancement

   on Jul 25, 2018
2. danwos

   self-assigned this

   on Jul 25, 2018
3. danwos commented on Jul 25, 2018

   

   danwos

   on Jul 25, 2018

   MemberAuthor

   More actions

   Maybe this kind of detailed definition is even not really needed.
   I see no use case, where an additional option should be something different as directive.unchanged.
   I guess that also our code would have some problem with any other kind of directive type...

   So needs_extra_options could be a simple list and sphinx-needs sets the needed directive type internally:
   needs_extra_options = ["author", "creation_date"]
4. tlovett commented on Jul 25, 2018

   

   tlovett

   on Jul 25, 2018

   Contributor

   More actions

   This model is rather restrictive though. For instance, if I want to introduce a piece of metadata and make it required, then I can't do that. Likewise if I want to introduce an attribute that is a flag. This seems like an important hook point for validation and error checking. The full set of built-in options conversion functions are here: http://docutils.sourceforge.net/docs/howto/rst-directives.html#option-conversion-functions however at they simple functions and there's no need to use those vs. your own.

   Improving the docs makes sense, but I wouldn't be in favor of removing the capability. Perhaps, if you want to make it easier, then we could change the default behavior to if you specify None then we default to unchanged. That avoids requiring an import, but leaves the option for customization if needed.

   needs_extra_options = {
      "author": None,
      "approved": directive.flag,
   }

   Thoughts?
5. danwos commented on Jul 25, 2018

   

   danwos

   on Jul 25, 2018

   MemberAuthor

   More actions

   No problem to keep it as it is, if there are cases for different options conversion functions.
   However I'm not sure if sphinx-needs can already handle it.
   For instance if positive_int_list shall be used, sphinx-needs may crash as it expects a string and not a list. Maybe the same for a flag (But I haven't check the code right now).

   So let's just do the doc update....
6. danwos commented on Sep 28, 2018

   

   danwos

   on Sep 28, 2018

   MemberAuthor

   More actions

   Fixed in 0.3.0

   👍React with 👍1Reacted by Trevor Lovett
7. danwos

   closed this as completedon Sep 28, 2018
8. chrisjsewell

   added a commit that references this issue on Sep 6, 2026

   🔧 Import sphinx-codelinks into the workspace as packages/sphinx-codel…

   ...

   a3aebf1