search

useblocks / sphinx-needs-vscode

https://sphinx-needs-vscode.useblocks.com
MIT License
3 stars 2 forks source link

fix: auto-generate specific IDs #38

Closed Chrizey91 closed 9 months ago

Chrizey91 commented 11 months ago

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 commented 10 months ago

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 commented 10 months ago

I think we should separate this PR: 1) Just the fix for #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 commented 9 months ago

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. :+1: