Improve external role warnings (and revert object fallback)#12193
Improve external role warnings (and revert object fallback)#12193chrisjsewell merged 14 commits intosphinx-doc:masterfrom
external role warnings (and revert object fallback)#12193Conversation
external role warnings more helpful (revert object fallback)external role warnings (and revert object fallback)
|
I do like the suggestion approach. It doesn't implicitly choose but rather suggest so I think it's a good approach. Now, I would go even further by suggesting the one of close Levenshtein distance.
Let's keep it backward compatible as much as possible. So keep the methods but make them deprecated. |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
…ell/sphinx into external-suggest-roles
done |
good idea, but can I delay this to another PR, it seems a bit more "involved" 😅 |
We can open an issue for that. |
yeh I could certainly see this also being applicable to other areas of reference resolution 😄 |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz
left a comment
There was a problem hiding this comment.
Those are the final nitpicks I can think of. Otherwise it's good for me. I'd be happy to work on the suggestion logic if you don't want to btw.
|
I like this solution, it keeps the core logic strict while helping users. Could the functionality be applied to role lookup in general? E.g., if you write |
great 😄
good idea, but if you don't mind, I would delay that to a separate PR |
Sure, though perhaps it could already be moved to another file as preparation? |
😬 I'd prefer to keep this PR "self-encapsulated", and not prematurely generalise (and get in to questions about what the file would be etc). |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
@picnixz since you marked your recent comments as resolved, I assume these are ok now? |
|
@chrisjsewell Bit of unrelated, but in general:
|
thanks for asking @picnixz
I guess whatever you have time for in the moment;
yeh this is fine, feel free to press the update Also, if you have any requests for me, feel free to say 😄 |
|
@chrisjsewell I've just noticed this PR added several new deprecations but didn't document them as deprecated -- please could you resolve? A |
external role warnings (and revert object fallback)external role warnings (and revert object fallback)
Ok @jakobandersen @picnixz, this is compromise for discussion in https://github.com/orgs/sphinx-doc/discussions/12152 😄
The key issue this seeks to address, is that existing tools / documentation often lead users to mistakenly use object types and not role names, a classic example being
functionnotfuncPreviously, the warning message for using e.g.
external:function`target`(withpyas the default domain), would be:This gives no information to the user on how to fix the issue, even though there is actually quite an easy fix
In #12133, I handled this by falling back to looking for the object type, then obtaining a role from that.
There has been some push back on this fallback 😬
So, in this PR, I revert that, but instead, create much more specific / helpful warning messages, e.g.
This goes through the same original logic but, if the role is not found, it will look if the role name is actually an available object type on the domain(s), and then suggest its related roles.
Note, in doing this, I've removed some methods of(added back and deprecated)IntersphinxRolebecause they are just not useful any more.