Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code documentation support #471

Open
wants to merge 33 commits into
base: glad2
Choose a base branch
from

Conversation

paoloose
Copy link

@paoloose paoloose commented May 18, 2024

This PR adds support for optional code documentation as requested in #247.

Basically, Specification classes can now include documentation from a language-agnostic SpecificationDocs class, which knows how to retrieve and parse the specs documentation from a specific source.

As a proof of concept I've implemented the DocsGL(SpecificationDocs) class for the GL specification, using https://docs.gl/ as source. docs.gl was definitely not created to be parser-friendly, but I managed to properly parse all the commands docs (without including tables and code blocks for now).

Additional sources will be needed for others specifications. Currently I'm only planning to contribute with the GL spec docs. Others can be added later with no problems.

Looking forward to feedback and willing to implement any requested modification. Thanks!

Test with:

# Use the --with-docs flag
python -m glad --extensions="" --api="gl:compatibility=" --out-path ./temp --with-docs c

Thinks to know:

  • Currently, only commands are documented.
  • Documentation code is optional and included only when the --with-docs flag is present. If the given specification doesn't support docs, only a warning is shown.
  • Documentation was only added to C templates, but can be easily added to Rust templates also.

@paoloose
Copy link
Author

image

This is how hovering a command looks in vscode

Forgot to add spaces when listing ('-') <dt> elements.

Also, the regex was improperly removing contiguous line breaks.
@Dav1dde
Copy link
Owner

Dav1dde commented May 20, 2024

Thanks for putting in the work, initial thoughts:

  • I'd prefer if it not depended on external tools and call processes with subprocess, it should be possible to just download a zip/tar file instead.
  • I wouldn't want to depend on docs.gl but instead the Khronos repo OpenGL-Refpages (except if there is a good reason to depend on docs.gl).
  • I thought this could be a maintenance nightmare but the docs don't seem to be changed very often (probably different for Vulkan ...)

glad/documentation.py Outdated Show resolved Hide resolved
glad/documentation.py Outdated Show resolved Hide resolved
glad/documentation.py Outdated Show resolved Hide resolved
glad/documentation.py Outdated Show resolved Hide resolved
glad/documentation.py Outdated Show resolved Hide resolved
glad/parse.py Outdated Show resolved Hide resolved
glad/parse.py Show resolved Hide resolved
@@ -1453,3 +1478,52 @@ def from_element(cls, element, api=None):
def __str__(self):
return '{self.name}@{self.version!r}'.format(self=self)
__repr__ = __str__


class SpecificationDocs(object):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this fits better in the documentation module

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I based the documentation,py module on how the specification.py is structured: just classes that inherit from the base Specification class defined in parse.py. So documentation.py holds specialized classes that inherit from the base SpecificationDocs class, also defined in parse.py.

glad/parse.py Outdated Show resolved Hide resolved
glad/parse.py Outdated
@@ -732,6 +747,15 @@ def select(self, api, version, profile, extension_names, sink=LoggingSink(__name
extensions = [self.extensions[api][name] for name in extension_names
if name in self.extensions[api]]

# Load documentation for the specific configuration
if config['WITH_DOCS']:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the specification doesn't need the docs at all. Instead of tying the docs to the specification, the templating could be the part where the specification gets married with the docs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please elaborate on this? The specification doesn't depend on the docs, but the docs need to know what specification are they documenting, and specifications need to know what documentation sources they have available, for example, if we support multiple sources in the future.

How can I define this relation in the templating process?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not quite sure about how I'd like it to look at the end, don't worry too much about it, it's easy for me to just move everything a bit around.

But basically in mind right now the docs are a separate source of data independent of the spec and you just pass it as some kind of "docs db" into the template and the template tries to lookup the docs from the db when rendering.

What guarantuess you get the correct symbols? Either it's in the template, a similar mechanism to spec.select() or just nothing, if you pass the wrong docs then there are just no docs for the requested symbols.

But again don't worry too much about that, these are the things that are easy to change later, the hard part is the actual logic!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this logic and now docs are loaded separately, only for the specifications listed by the user. Then, those loaded docs sources are "selected" from a feature_set, and passed directly to the template generator function, without depending on the spec. I think I ended up understanding what you meant, but if not, feel free to change it later. I'll be working on using the Khronos Refpages as source :-)

@paoloose
Copy link
Author

I wouldn't want to depend on docs.gl but instead the Khronos repo OpenGL-Refpages (except if there is a good reason to depend on docs.gl).

Seems good to me. I'll be replacing DocsGL with that source. I understand that docs.gl is more a community driven documentation than a strict reference. I was also consider shipping both, but from a maintainer point of view docs.gl is not convenient.

Working on resolving the other requested changes :)

@Dav1dde
Copy link
Owner

Dav1dde commented May 22, 2024

I understand that docs.gl is more a community driven documentation than a strict reference.

I assumed it is based on the same OpenGL-Refpages repo just already processed for better display on docs.gl but without additional curation.

@paoloose
Copy link
Author

I assumed it is based on the same OpenGL-Refpages repo just already processed for better display on docs.gl but without additional curation.

See https://docs.gl/about.html. It adds additional information to the official specs (even typo fixes) and allows contributions, but yeah, what was said about maintenance still holds. Still working on parsing Khronos reference.

Copy link
Owner

@Dav1dde Dav1dde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting in all the work and sorry for being so unresponsive slow, I just don't have the mental bandwidth atm.

glad/documentation.py Outdated Show resolved Hide resolved
glad/documentation.py Outdated Show resolved Hide resolved
glad/parse.py Outdated Show resolved Hide resolved
glad/parse.py Outdated Show resolved Hide resolved
glad/__main__.py Outdated Show resolved Hide resolved
- load lxml module first, and fallback to xml module
- don't extract zip file to parse the documents
- fix other formatting issues that I encountered
@paoloose
Copy link
Author

paoloose commented Jun 3, 2024

Thanks for putting in all the work and sorry for being so unresponsive slow, I just don't have the mental bandwidth atm.

No worries, thank you for taking the time to review it. I'm dogfooding this docs while I'm working in some OpenGL projects, so I'll be pushing fixes as I encounter more issues. For now I've implemented the requested changes. Wish the code was more elegant, but Khronos really worked hard to make their documentation format a parsing nightmare haha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants