readthedocs / sphinxcontrib-dotnetdomain

A Sphinx domain for .NET languages
MIT License
15 stars 17 forks source link

Intersphinx object inventories don't load properly into dotnet domain #46

Closed agjohnson closed 8 years ago

agjohnson commented 8 years ago

Intersphinx mappings load inventories into the domain incorrectly. Test this more.

agjohnson commented 8 years ago

This seems to be due to the way that Sphinx handles domain object roles between the ObjectType class and Domain class instantiation. When multiple roles are defined for a single object type, roles for the Domain class are set incorrectly, due to the assumption that there will be only one role for an object type. This isn't the case based on how object can be defined however.

ericholscher commented 8 years ago

LGTM, though I'm not sure about it solving the actual problem. I'm guessing testing this is hard? We could perhaps generate the intersphinx inventory and then process it to test it round trips properly?

agjohnson commented 8 years ago

Yeah, was not sure how to best write a test for this. Perhaps mocking the intersphinx load function and testing a link is resolved? Seems doable

ericholscher commented 8 years ago

You could probably run the actual code and validate it -- https://github.com/sphinx-doc/sphinx/blob/master/sphinx/ext/intersphinx.py#L79

agjohnson commented 8 years ago

Yeah, though I'd probably just skip that step to avoid mucking around with their compressed inventory file format, it's a pain. Mocked an actual object would be much easier

agjohnson commented 8 years ago

Tests attached. I wrote a mocked XML builder to provide XML string output. The text output doesn't actually output a linked syntax, only the literal that's quoted. The XML outputs a full link that we can inspect actually resolved.

ericholscher commented 8 years ago

:+1: Looks good