sphinx-needs icon indicating copy to clipboard operation
sphinx-needs copied to clipboard

Needs list shall take into account "only" directive

Open David-Le-Nir opened this issue 1 year ago • 11 comments

When using the only directive some needs may require to be excluded from the needs list, i.e. from needs.json, needtable, needpie ...

For example:

.. only:: tag1

  .. req:: requirement for tag1
       :id: req-001

       This one shall only appear when building tag1

.. req:: requirement for all tags
     :id: req-002

       This one shall always appear

in this case when I don't build tag1, I don't want req-001 to appear in needs.json or in needtable

Do you agree about the expected behaviour ? Do you know if while processing the need level we are able to know that the element is under an "only" block ?

just for information, we found this interesting extension which can be inspiring: https://github.com/pfalcon/sphinx_selective_exclude

David-Le-Nir avatar Feb 08 '24 13:02 David-Le-Nir

Heya, oo tricky one

So, setting aside the logic of how to actually use this data for now, I think we may be able to capture the only tags on the needs data item, within the analyse_need_locations function.

Here you see we already capture what parent sections the need is under, so we may be able to do the same thing to capture what parent only the need is under:

https://github.com/useblocks/sphinx-needs/blob/4faedec27c24e314b7da9c298dc9aa67e97e9ff6/sphinx_needs/directives/need.py#L288

chrisjsewell avatar Feb 08 '24 14:02 chrisjsewell

would removing the nodes as it is done for hidden needs be sufficient ? or it is only about the display ?

David-Le-Nir avatar Feb 09 '24 10:02 David-Le-Nir

would removing the nodes as it is done for hidden needs be sufficient ? or it is only about the display ?

Only about display I feel #1106 misunderstands hidden needs: they don't remove the need data, merely its represenataion in the output (e.g. in the HTML)

Remove data at this stage is not viable, because it would not work for multiple (cached) builds, using different tags / builders, i.e. subsequent builds would always have this data removed, even if you change the tags

The cached data (from the read phase) needs to be agnostic of builder / tags, hence why I said you need to store the only expressions in the need data item. Then, later in the process, during the builder/tag specific write phase, you decide what needs to omit

chrisjsewell avatar Feb 09 '24 17:02 chrisjsewell

Thanks for the explanation. I started a PR but, I don't know why, I do not manage to run tests.

Anyway, adding a field to the needs is ok. I propose to add boolean field named "excluded_by_only". I'm not very happy with this name but could not find a better one. Please advise for another name if you have a better one.

I think excluding from the html builder is already handled. I'll look how to exclude from the needs builder and also for directive like needtable, needpie, needlist. Maybe there is one place to rule them all ?

David-Le-Nir avatar Feb 10 '24 08:02 David-Le-Nir

I somehow hate the only directive, as all the content gets rendered every time and gets then removed later by the builder. That also makes trouble for directives like toctree and others.

For me, the builder, somewhere in Sphinx, or the only directive would be the correct place to solve this. But I guess this will never happen :/

For the above reason, I needed to add an if-builder directive to sphinx-simplepdf. Its content gets not rendered and therefore is working with sphinx-needs and toctree. https://sphinx-simplepdf.readthedocs.io/en/latest/directives.html#if-builder

I also see a lot of corner cases, if the need-directive is embedded inside other needs (list-tables, container, other needs, ...). There can be so many, and checking all the node tree upwards may cost some performance.

I'm currently not sure what the best solution is.

danwos avatar Feb 12 '24 08:02 danwos

Do you mean if the if-builder accepted tags it could greatly replace the only ? (I didn't check the code, sorry)

Maybe adding the boolean field at the need level is an acceptable first step for the moment as it will allow to manually filter in needs.json or needtables and so on. I just have to ensure it is correctly computed for nested containers.

But this is not very user friendly and should be documented so that users do not forget to manually filter this boolean field when they are using the only directive.

edit: another possibility, instead of the boolean would be to copy the tags from the only level to the need, so manually filtering would be more comprehensible for user. Either by appending the the tags field (which content will be displayed, so maybe not the better for user) or by adding a new field.

edit-2: after some trials I decided to be more radical by removing the needs from the needs_all_needs and the rendering node. I also tried to manage nested needs. The unit tests shows that needs are excluded from needs.json and from directives which is the expected behavior (at least from y point of view ^^) But from your explanations I guess I didn't handled well all the complexity behind ?

David-Le-Nir avatar Feb 12 '24 11:02 David-Le-Nir

Will hopefully find some time to review the PR again soon.

In the meantime, yes if-builder is often used as an replacement for only, and in my experience it works much better. But I wasn't aware only supports sphinx-tags, so that is a missing feature.

Maybe a small, new sphinx extension Sphinx-If could be the better solution. Taking the code from if-builder, extending it to support sphinx-tags, and it's done :) Supporting Sphinx-Needs objects, toctree and co. out of the box, by not even parsing not needed constructs.

Regarding edit-2: I guess this is the way to go. Only having the data inside needs.json, which shall be there. No magic boolean value, which needs to be known and handled correctly.

danwos avatar Feb 15 '24 07:02 danwos

Maybe a small, new sphinx extension Sphinx-If could be the better solution. Taking the code from if-builder, extending it to support sphinx-tags, and it's done :)

Do you think we have to create a new, independent, sphinx extension of just improve the if-builder to accept tags ? Maybe a new extension would be too much duplicating ? Or maybe if-builder and if-include should be entirely moved to sphinx-if ?

I have to discuss with my teammates but as only is an important feature for our users we could take time to create this directive. Or would you prefer to handle it on your own ? (If we do it, it will be somehow a big copy-paste from your work)

David-Le-Nir avatar Feb 15 '24 08:02 David-Le-Nir

Technically we could do it everywhere, but using an updated if-builder would always mean installing the complete sphinx-simpldpdf package. So 99% of code and dependencies would not be needed.

So, I guess a small standalone Sphinx extension would be much cleaner. I would also then remove if-builder from Sphinx-SimplePDF one day and reference to the more generic solution.

And I would be grateful if you could support me here and create the initial extension. For sure copying all the needed code is fully ok, and me and my small team can also support or add single features as PR, if there are special use cases from our existing packages.

danwos avatar Feb 15 '24 08:02 danwos

Hello,

I implemented (for the moment locally in our gitlab, not on github) the "only-tags" and "only-builder" directives based on what was made in sphinx-simplepdf. This works pretty well but I have an issue and maybe you could have an idea about how fixing it:

If I do for example:

.. only-builder:: text

   .. toctree::

      file_text.rst
      file_all.rst

.. only-builder:: html

   .. toctree::

      file_html.rst
      file_all.rst

I get the right toctrees and navigation links in the generated documents based on different builders. However the "file_text" and "file_html" are always built whatever the chosen builder. Thus if I perform a search, I get them in the results and I'm able to navigate on it.

Do you have an idea of how I can exclude them from generation or else from search result ?

Regards

David-Le-Nir avatar Mar 26 '24 17:03 David-Le-Nir

Argh, that may be tough, as Sphinx generates automatically all rst/md files it finds in the used source-dir, even if they are not part of a toctree.

I'm not sure if exlude_patterns could be set programmtically.

danwos avatar Apr 02 '24 20:04 danwos