Skip to content

Conversation

@juiwenchen
Copy link
Contributor

@juiwenchen juiwenchen commented Sep 23, 2025

Issues

#23
#15

Summary

Based on the issues, adapt the directive src-trace to have id field, so that the config id_required=True does not block the need generation. This adaptation react to needs config id_from_title and id_length

Implementation

  • id is optional and when it's not given, the hash is always generated from titile and project. If directory or file is given, it's taken as well.
  • added integration test cases for id_required
  • emit error msg when Sphinx-Needs is not set as extensions

@juiwenchen juiwenchen requested a review from ubmarco September 23, 2025 11:49
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.

Can you make this optional, so if users do not provide the ID, you calculate it yourself from the project/directory/file (replace all non-identifier chars with _). Or calculate a short hash from it, just like SN does. Something reproducible with the same inputs.
That ID information will presumably not be helpful because why would people link to the src-trace directive itself?
But if we want src-trace to be a dedicated need, then we should do that. Forcing users to set IDs that will not be useful is meeh.

A src-trace for the same combination of project, directory, and file cannot exist in a Sphinx project because that leads to duplicate needs.

See also #15 (comment)

@juiwenchen juiwenchen requested a review from ubmarco September 25, 2025 12:05
@ubmarco ubmarco changed the title Fix id reqired 🐛 Handle id_required for src-trace Sep 25, 2025
@juiwenchen juiwenchen merged commit 2412820 into main Sep 25, 2025
8 checks passed
@juiwenchen juiwenchen deleted the fix-id-reqired branch September 25, 2025 13:37
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.

3 participants