sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.56k stars 2.12k forks source link

Use more descriptive Python domain objtype values #10617

Open jbms opened 2 years ago

jbms commented 2 years ago

As of https://github.com/sphinx-doc/sphinx/issues/3743 Sphinx no longer uses the classmethod or staticmethod objtypes, even though they are still defined by the Python domain.

I agree that the new syntax for defining such methods is an improvement:

.. py:method:: Foo.bar(cls, a, b)
   :classmethod:
   :async:

However, along with that change, the more descriptive objtype values were also lost, which I think is unfortunate. In sphinx itself, the objtype allows for more descriptive search results. Additionally, extensions such as https://github.com/jbms/sphinx-immaterial heavily rely on the objtype to allow the user to specify a variety of objtype-specific customizations.

I am proposing that we retain the existing rST syntax, but change the objtype to include all of the options, e.g. asyncclassmethod, etc.

If this feature sounds acceptable, I can introduce a PR.

There is a potential issue that extensions or conf.py files that rely on the existing objtype values could be broken, though. However, I suspect direct uses of the objtype values is relatively rare, outside of the sphinx-immaterial extension.

AA-Turner commented 2 years ago

Could you add some colour to the breaking change possibility? This sounds like a good idea, if the risk of breaking change is near-vanishing we should just go ahead, if not we could consider a feature-flag approach.

A

jbms commented 2 years ago

I don't really have any idea how much of a breaking change it would be --- it would probably require testing to figure that out --- maybe the testing procedures already done for Sphinx would help with that.

There is also some question as to which options should be incorporated into the objtype.

For example, staticmethod vs method is definitely very relevant for users of an API. staticmethod vs classmethod is relevant if extending the class, otherwise not very relevant. async is a bit unclear, because in a sense it is just a property of the function's implementation --- the return type already indicates the relevant information to users regardless of how it is actually implemented.

AA-Turner commented 2 years ago

async is a bit unclear, because in a sense it is just a property of the function's implementation --- the return type already indicates the relevant information to users regardless of how it is actually implemented.

Return types aren't mandatory though, so the distinction could be useful.

A

tk0miya commented 2 years ago

What about abstractmethod? What about final? I think it's difficult to use composite objtypes because of combinatorial explosion. This is why we switched to using the directive options to describe functions and methods. So it's better not to separate objtypes. Let's add an extra field to object entry to describe the kind of functions and methods.

AA-Turner commented 2 years ago

Hmm, I was thinking about core Python builtins---if we expand to abc or typing.final then yes it becomes a more dubious proposition.

@jmbs if we included a flags or props or whatever attribute, would that be usable in immaterial?

A

tk0miya commented 2 years ago

Python domain already supports abstractmethod and final. https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#directive-py-method

jbms commented 2 years ago

Yes I agree that a separate flags/properties field would be better, and that could work for the sphinx-immaterial theme. I suppose the model would be that each object type could define a list of associated optional flags:

Then in search results maybe all of the flags would be shown in order for each result (order determined by order in which flags are defined)