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

RFC: Cache filename #156

Closed
wants to merge 2 commits into from
Closed

RFC: Cache filename #156

wants to merge 2 commits into from

Conversation

jnikula
Copy link
Owner

@jnikula jnikula commented May 24, 2023

Only tested locally, no documentation, but thoughts?

Follow the hawkmoth naming more consistently with
s/cautodoc_parsed_files/hawkmoth_parsed_files/
When documenting lots of symbols from a file, it gets tedious to repeat
the :file: option:

.. c:autofunction:: foo
   :file: baz.c

.. c:autofunction:: bar
   :file: baz.c

Cache the filename from the :file: option across directives within a
document.
@jnikula
Copy link
Owner Author

jnikula commented May 25, 2023

Perhaps better ideas in #157

@jnikula jnikula marked this pull request as draft May 25, 2023 08:56
if filename:
self.env.temp_data['hawkmoth_current_file'] = filename
else:
filename = self.env.temp_data.get('hawkmoth_current_file')
Copy link
Collaborator

@BrunoMSantos BrunoMSantos May 28, 2023

Choose a reason for hiding this comment

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

Right, so having looked into this I stand by my comment in #157: I don't like the idea of stateful directives too much. But I've also reconsidered things a bit, even if I would still personally vote this particular implementation down in favour of having a specialized directive to select a file.

Not only that would be much more explicit, we could later combine it with full lookup / cache as a way of speeding things up by limiting the search to a (set of) directory --- i.e.: .. autoresolve:: some/dir. And in that case, adding the new directive with support for a single file only could be done without implying future breaking API changes.

Thoughts?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Right, so having looked into this I stand by my comment in #157: I don't like the idea of stateful directives too much. But I've also reconsidered things a bit, even if I would still personally vote this particular implementation down in favour of having a specialized directive to select a file.

Agreed. If there are going to be stateful directives, they should be explicit rather than implicit like this.

@BrunoMSantos BrunoMSantos mentioned this pull request Jul 9, 2023
@jnikula
Copy link
Owner Author

jnikula commented Jul 10, 2023

Superseded by #168

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