Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add list-needs directive #1427

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

✨ Add list-needs directive #1427

wants to merge 1 commit into from

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Mar 24, 2025

The list2need directive has a number of notable issues:

  • It uses a bespoke syntax that users have to learn, and contains numerous problematic edge cases (need2list results in incorrect source mapping #1323)
  • It uses jinja templating for parsing, which
    • breaks the source mapping for the entire file a list2need is part of
    • it is incompatible with other parsers such a MyST

This directive is intended to replace list2need, using parsing logic similar to the list-table directive, whereby the content is first parsed as a standard field list, then converted to needs.
This contains no bespoke syntax, preserves source mapping, and can be used within a MyST document.

See https://sphinx-needs--1427.org.readthedocs.build/en/1427/directives/list-needs.html

TODO: if agreed, look to "soft deprecate" list2need, so that at least new users don't use it, and old users can be guided to start porting


Comparision:

.. list2need::
   :types: req, spec, feat
   :tags: a,b,c

   * Need example on level 1
   * (NEED-002) Another Need example on level 1 with a given ID
     * Sub-Need on level 2 with status option set ((status='open'))
     * Another Sub-Need on level 2. Where this sentence will be used
       as content, the first one as title.
       * Sub-Need on level 3. With some rst-syntax support for
         the **content** by :ref:`list2need`
.. list-needs::
   :defaults:
      :tags: a,b,c

   :req: Need example on level 1
   :req id=NEED-002:
      Another Need example on level 1 with a given ID

      :spec status=open: Sub-Need on level 2 with status option set 
      :spec: Another Sub-Need on level 2.

         Where this sentence will be used as content, the first one as title.

         :feat collapse: Sub-Need on level 3.

            With all rst-syntax support for the **content** by :ref:`list-needs`

Most basic example:

.. list-needs::

    :req: Need example title
    :req: Another need example with nested needs.

       :spec: Sub-Need on level 2
       :spec: Another sub-need

Copy link

codecov bot commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 80.47809% with 49 lines in your changes missing coverage. Please review.

Project coverage is 88.26%. Comparing base (4e10030) to head (f8c0553).
Report is 123 commits behind head on master.

Files with missing lines Patch % Lines
sphinx_needs/directives/listneeds.py 79.14% 49 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1427      +/-   ##
==========================================
+ Coverage   86.87%   88.26%   +1.38%     
==========================================
  Files          56       61       +5     
  Lines        6532     7471     +939     
==========================================
+ Hits         5675     6594     +919     
- Misses        857      877      +20     
Flag Coverage Δ
pytests 88.26% <80.47%> (+1.38%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chrisjsewell chrisjsewell requested review from ubmarco and danwos March 24, 2025 09:48
Copy link
Member

@ubmarco ubmarco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very useful improvement.
In addition to my comments I would directly deprecate list2needs.

Comment on lines +89 to +90
:links-down: blocks, triggers
:links-up: tests, checks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the example below, LIST-l2b triggers LIST-l3 and LIST-l3 checks LIST-l2b.
These are 2 different link types, while SN offers back-links for this.
If links-up and link-down are independent from each other and one can be left out, I'm ok with this.
Then it's up to the developer to decide how they want to link. And whether they want 2-sided links with different types (and different back-links each).

``maxdepth``
~~~~~~~~~~~~

The ``maxdepth`` option allows you to limit the depth of converted field lists.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please state the consequence if a nested item is deeper than the given maxdepth.

@danwos
Copy link
Member

danwos commented Mar 24, 2025

The main motivation of list2need was to have a short, simple syntax to create quick & dirty lists during meetings.

The new one is quite overhanging for this use case.

I like the implementation itself, but for the syntax we should try to keep it comprehensive and short. For instance the list-level shall automatically define the need-type.

So the question is, can we save the syntax (mostly) but have a better implementation (without jinja)?

@ubmarco
Copy link
Member

ubmarco commented Mar 24, 2025

In its most basic form, the new directive looks like this:

.. list-needs::

    :req: Need example title
    :req: Another need example with nested needs.

       :spec: Sub-Need on level 2
       :spec: Another sub-need

I think this is also comprehensive and short. (I updated above example, it's messed up due to tabs/spaces mix.)
Only diff to list2needs is that the need type is explicitly specified. Cannot let the nesting level decide about that anymore.

We win

  • standardized syntax that is part of the RST language
  • syntactical separation of options (part of the field) and title markup (right of the field); this also means no custom syntax and no escaping mechanisms around brackets.
  • no special symbol . that separate title and content; while I see the point of having the smallest possible syntax, it's hard to build a well-behaving parser around this (how to end a title with a dot?) without thinking about all the corner cases.
  • avoid Jinja templating (which we could also fix in the list2needs directive)

We lose:

  • list2needs users have to migrate their syntax - not sure if that is a heavily used feature; with a deprecation phase I think this is acceptable.

Any other opinions from the community?

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Mar 24, 2025

Any other opinions from the community?

I would cc @christopheseyler, as this would essentially supersede #1416 (and contains the defaults option)

@christopheseyler
Copy link
Contributor

christopheseyler commented Mar 25, 2025

I do not see the benefit comparing to the existing : the list2need provides a lightweight and more direct syntax to quickly write a list of requirements

.. list-need
    :req: Need example on level 1

    :req id=NEED-002: Another Need example on level 1 with a given ID

instead of

.. need:: Need example on level 1

.. need:: Another Need example on level 1 with a given ID

    :id: NEED-002

The syntax of the actual list2need is more compact and more readable, IMO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants