Improve external role warnings (and revert object fallback)

[chrisjsewell]

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 function not func

Previously, the warning message for using e.g. external:function`target` (with py as the default domain), would be:

WARNING: role for external cross-reference not found: function

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.

WARNING: role for external cross-reference not found in domains 'py', 'std': 'function' (perhaps you meant one of: 'py:func', 'py:obj')

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 IntersphinxRole because they are just not useful any more. (added back and deprecated)

[picnixz]

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.

Note, in doing this, I've removed some methods of IntersphinxRole because they are just not useful any more.

Let's keep it backward compatible as much as possible. So keep the methods but make them deprecated.

[chrisjsewell]

Let's keep it backward compatible as much as possible. So keep the methods but make them deprecated.

done

[chrisjsewell]

now, I would go even further by suggesting the one of close Levenshtein distance.

good idea, but can I delay this to another PR, it seems a bit more "involved" 😅

[picnixz]

good idea, but can I delay this to another PR, it seems a bit more "involved" 😅

We can open an issue for that.

[chrisjsewell]

We can open an issue for that.

yeh I could certainly see this also being applicable to other areas of reference resolution 😄

[picnixz]

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.

[jakobandersen]

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 :external:function: or just :function: it would do the same thing?

[chrisjsewell]

I like this solution, it keeps the core logic strict while helping users.

great 😄

Could the functionality be applied to role lookup in general? E.g., if you write :external:function: or just :function: it would do the same thing?

good idea, but if you don't mind, I would delay that to a separate PR

[jakobandersen]

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?

[chrisjsewell]

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).
The code in this PR does not introduce any new public API, so I feel there will be no problem in a future PR if the code needs to be moved

[chrisjsewell]

@picnixz since you marked your recent comments as resolved, I assume these are ok now?

[picnixz]

@chrisjsewell Bit of unrelated, but in general:

• do you prefer that I review bit by bit, possibly addressing different points in different reviews or everything in one go?
• do you prefer that I let you update your branches if I push on master or can I press the "update branch" button if there are no conflicts? (I will not solve conflicts myself since you might have local changes not yet committed).

[chrisjsewell]

Bit of unrelated, but in general:

thanks for asking @picnixz

do you prefer that I review bit by bit, possibly addressing different points in different reviews or everything in one go?

I guess whatever you have time for in the moment;
if you only have time for a quick partial review, thats fine and its good to get early feedback (at least whether you think the PR is a good idea in principle),
but maybe just mention that there will probably be more to come

do you prefer that I let you update your branches if I push on master or can I press the "update branch" button if there are no conflicts? (I will not solve conflicts myself since you might have local changes not yet committed).

yeh this is fine, feel free to press the update

Also, if you have any requests for me, feel free to say 😄

[AA-Turner]

@chrisjsewell I've just noticed this PR added several new deprecations but didn't document them as deprecated -- please could you resolve?

A