👌 Capture `only` expressions for each need

[chrisjsewell]

@David-Le-Nir and @danwos, as I explained in https://github.com/useblocks/sphinx-needs/issues/1103#issuecomment-1936305902, I think this is a better solution for handling need defined within only directives.

In this first "read" phase, we simply just note all the parent only expressions of the need, storing them on an (optional) only_expressions field of the need data item.

If desired, in a subsequent "build" post-processing phase, called from the cached data (once per build), you could then evaluate the only_expressions and remove needs as a necessary (or do whatever). This logic would go here: https://github.com/useblocks/sphinx-needs/blob/84a5f72f2e72ab1471ab2d1bb5c570d6115ef199/sphinx_needs/directives/need.py#L385

this is more in-line with the logic of the only directive, where everything is cached and parts of the doctree are only removed during the build phase.

would supercede #1106

closes #1103

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (84a5f72) 83.93% compared to head (04e99a9) 83.96%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1112      +/-   ##
==========================================
+ Coverage   83.93%   83.96%   +0.02%     
==========================================
  Files          56       56              
  Lines        6462     6473      +11     
==========================================
+ Hits         5424     5435      +11     
  Misses       1038     1038              

Flag	Coverage Δ
pytests	83.96% <100.00%> (+0.02%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[chrisjsewell]

I see that the only_expressions is shown in the rendered admontion, I guess this should be hidden:

[David-Le-Nir]

Hello,

I first tried as you said but I was annoyed that I'd have to take this new field into account in each directive (e.g. needtable) to exclude the need automatically or that users would have to manually exclude the needs when using the directives. That's why I decided to be more brutal but I read again your comment in #1103 and it's now clear for me ^^.

About the post-processing, do you mean I have to implement my own logic in my own document ? Or do you suggest we add to this PR an exclusion of the needs in post_process_needs_data ?

[danwos]

I like the PR from a technical point of view, but I'm not a fan of it from a usability point of view.

I agree with @David-Le-Nir, that adding this to filter-strings and Co, to get a correct result, is not the best way to go. 95% of the users will likely not be aware of this.

Especially in bigger projects a user may not know that only is used somewhere.

In most of bigger user projects, we decided to always use a full build to get a reliable result for a Sphinx-Needs project, as there can be so much external data and dynamic functions, which do not work well with an incremental build. Maybe using only is also such a case, so the user needs to be aware to create a full-build, if the final docs shall be used somehow official.

Not carrying correctly about only_expressions in filter strings could result every time in an invalid result. Imagine having 2 Sphinx-Needs objects, one for html and one for pdf. If the filter string does not contain only_expressions, you will always get 2 items as a result.

Removing only-needs from the needs-dict may not be perfect, but at least we have one setup, which gives us always a valid result, by running simply a full build.

[chrisjsewell]

we decided to always use a full build to get a reliable result for a Sphinx-Needs project

is this in the documentation?

If it doesn't, then really thats a quite a big limitation for sphinx-needs

as there can be so much external data and dynamic functions, which do not work well with an incremental build.

I feel that sphinx-needs in general works quite well with incremental. Certainly all the changes I have made, have been with that in mind, for example, dynamic functions are run after the needs-dict is cached, so these do work for incremental builds Where it doesn't then we should work to fix that.

I am really quite against introducing things like #1106, that inherently break incremental builds

Not carrying correctly about only_expressions in filter strings could result every time in an invalid result. Imagine having 2 Sphinx-Needs objects, one for html and one for pdf. If the filter string does not contain only_expressions, you will always get 2 items as a result.

I don't think this is correct; you would remove the needs items (that have negatively-evaluated only_expressions) before any filter strings are processed

[danwos]

I don't think this is correct; you would remove the needs items (that have negatively-evaluated only_expressions) before any filter strings are processed.

Ahh okay, then I have misunderstood the concept. But the current PR does not include this removal from the needs-list, right?

I feel that sphinx-needs in general works quite well with incremental. Yes, it technically works well and incremental builds are running smoothly.

But if you change or remove a Need object in File A, needtables and co do not get updated in File B. So the shown data is not valid. So the representation of changed need objects in tables, flow charts, and extracts may not be correct, if all the files containing such need-representing directives get not rebuild as well. So in the end, a full-build is needed.

[chrisjsewell]

Or do you suggest we add to this PR an exclusion of the needs in post_process_needs_data ?

Ahh okay, then I have misunderstood the concept. But the current PR does not include this removal from the needs-list, right?

yes and yes; I haven't implemented the removal yet, just wanted to intially show how the "first" phase would work

[chrisjsewell]

But if you change or remove a Need object in File A, needtables and co do not get updated in File B.

if doesn't already, there is no reason why it cannot be; the needtable is rendered in the build (a.k.a post-processing) phase, so will use the updated need data I think is just because FIle B is not being re-built, but I feel you could get this to trigger

[chrisjsewell]

So in the end, a full-build is needed.

but yeh obviously this is a more broad topic that I want to look into

It should also be noted that there are different "levels" of incremental builds, for example:

• sphinx-build -E will delete the cache, so that the entire read phase and the entire build phase are run
• sphinx-build -a will not delete the cache, so only new/changed files will be re-read, but it will enforce a re-build of all files during the write phase, so all changes to the needs data would be reflected. @danwos I think if you tried with this option, incremental builds for needtables would already work