sphinx-needs icon indicating copy to clipboard operation
sphinx-needs copied to clipboard

Allow to specify a part separator other than dot "."

Open holmboe opened this issue 10 months ago • 6 comments

This (partly?) solves #1160.

With this patch you can refer to a need containing a dot, an example:

:need:`A5.1`

Before this patch:

Traceback (most recent call last):
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/cmd/build.py", line 298, in build_main
    app.build(args.force_all, args.filenames)
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/application.py", line 351, in build
    self.builder.build_all()
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/builders/__init__.py", line 252, in build_all
    self.build(None, summary=__('all source files'), method='all')
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/builders/__init__.py", line 363, in build
    self.write(docnames, list(updated_docnames), method)
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/builders/latex/__init__.py", line 299, in write
    doctree = self.assemble_doctree(
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/builders/latex/__init__.py", line 365, in assemble_doctree
    self.env.resolve_references(largetree, indexfile, self)
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/environment/__init__.py", line 676, in resolve_references
    self.apply_post_transforms(doctree, fromdocname)
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/environment/__init__.py", line 693, in apply_post_transforms
    self.events.emit('doctree-resolved', doctree, docname)
  File "/home/nn/.local/pipx/venvs/sphinx/lib/python3.10/site-packages/sphinx/events.py", line 97, in emit
    results.append(listener.handler(self.app, *args))
  File "/home/nn/code/github.com/holmboe/sphinx-needs/sphinx_needs/needs.py", line 350, in process_caller
    check_func(app, doctree, fromdocname, current_nodes[check_node])
  File "/home/nn/code/github.com/holmboe/sphinx-needs/sphinx_needs/roles/need_ref.py", line 95, in process_need_ref
    dict_need["title"] = target_need["parts"][need_id_part]["content"]
KeyError: '1'
> /home/nn/code/github.com/holmboe/sphinx-needs/sphinx_needs/roles/need_ref.py(95)process_need_ref()
-> dict_need["title"] = target_need["parts"][need_id_part]["content"]

holmboe avatar Mar 31 '24 08:03 holmboe

Codecov Report

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

Project coverage is 85.92%. Comparing base (0f71ecd) to head (f9d3c87).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1159   +/-   ##
=======================================
  Coverage   85.92%   85.92%           
=======================================
  Files          56       56           
  Lines        6536     6538    +2     
=======================================
+ Hits         5616     5618    +2     
  Misses        920      920           
Flag Coverage Δ
pytests 85.92% <100.00%> (+<0.01%) :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.

codecov[bot] avatar Mar 31 '24 08:03 codecov[bot]

I have studied the code further and have identified a few more places where it would require changes for the config option needs_part_separator to have effect. Consider this PR a draft and place for discussion regarding a full implementation. It is likely that this PR will be closed and a new PR with full(er) support will take its place.

Thoughts?

holmboe avatar Apr 02 '24 12:04 holmboe

heya thanks, yeh I'm not against it in principle, but indeed would require a good look (and testing) to make sure it is working as expected 👍

chrisjsewell avatar Apr 02 '24 12:04 chrisjsewell

I like the idea, but I'm afraid that this change is a tough one, as . as seperator is somehow hard-coded at a lot of places.

So the next step should be to have 2-3 additional test cases, which are changing the seperator and tests, if filtering, needflows and co. are all working.

danwos avatar Apr 02 '24 20:04 danwos

Yes I can see that this is hard-coded in various places. I am motivated to dig through the code to find them out. However, I would be greatly helped if someone who is more intimately knowledgeable of the roles/directives/whatnot could create some test cases that is marked xfail. Any chance of this? @danwos @chrisjsewell

holmboe avatar Apr 03 '24 20:04 holmboe

Note to self: this is likely helped by and related to split_need_id() merged in #1101.

holmboe avatar Jun 20 '24 10:06 holmboe