Skip to content
This repository has been archived by the owner on Sep 26, 2024. It is now read-only.

fix: auto-generate specific IDs #38

Closed
wants to merge 2 commits into from

Conversation

Chrizey91
Copy link

This PR aims to fix issue #33 while simultaneously adding the minor feature of tabbing through the fields of a snippet (I think this was present at some earlier version, but was broken later on).

IMPORTANT: Within the needs.json-file the information about the prefixes configured in the conf.py is not present and consequently, I wrote a very simple algorithm that calculates the prefix from the given needs. In turn, the auto-generated prefex is not guaranteed to be identical to the prefix configured in the conf.py!

@haiyangToAI
Copy link
Contributor

Thanks for your interest in this extension.

As you have noticed, the issue is how to get the prefix of need type, since this info lies in conf.py. But this extension is based on needs.json, and it has zero knowledge of conf.py.

In your implementation, the prefix is assumed by finding common prefix of first two needIDs for each need type. To me, I don't find it so convincing.

However, thanks for your time and efforts trying to improve this extension. Cheers!

@danwos
Copy link
Member

danwos commented Jan 11, 2024

I think we should separate this PR:

  1. Just the fix for Snippet and auto id generation do not work as before #33
  2. The id_prefix guessing implementation.

I also have some concerns for the second one, as it could go wrong and then the proposed ID is always wrong.
Imagine an ID syntax that contains some grouping information at the beginning, maybe the component name or an ID of an external reference project.
I like the idea of being more specific in the proposed ID, but it should be more precise/correct.

In Sphinx-Needs, there was already the discussion to add some basic configuration information into the needs.json file, so that a later script/tool knows what requirements need to be fulfilled before importing the data.
I guess if this information is available, we can get the prefix from there.

@danwos
Copy link
Member

danwos commented Feb 15, 2024

I close this in favor of PR #40, which integrates only part 1) of the above comment.

Thanks for all the investigation and the discussions, it's helped a lot to clarify things and get a good impression about the problem. 👍

@danwos danwos closed this Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants