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

Improve pdoc rendering #68

Merged
merged 1 commit into from
Jun 5, 2021
Merged

Improve pdoc rendering #68

merged 1 commit into from
Jun 5, 2021

Conversation

mhils
Copy link
Contributor

@mhils mhils commented May 30, 2021

Hi there!

Thanks for the excellent project - it's a nice feeling to work with code that then again makes use of one's own project. 😄

I noticed that you have an interesting single-page configuration for pdoc which is slightly awkward with pdoc's folder structure expectations. I just pushed a new pdoc release which supports this a bit better, here are the accompanying changes for arxiv.py. 😃

@lukasschwab
Copy link
Owner

👋 Hi! Thanks for your work––pdoc is great.

Well-spotted. Maybe it's a mistake in how I designed this package, but pdoc's index wasn't super useful for this project––it just lists one module, arxiv, so I figured I'd pull that module's docs page out to replace the index. Not sure that this is typical, but glad to hear it's better supported now!

Reviewing...

@lukasschwab lukasschwab self-requested a review June 5, 2021 21:31
Copy link
Owner

@lukasschwab lukasschwab left a comment

Choose a reason for hiding this comment

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

LGTM! Merging...

Comment on lines -20 to -21
# Fixup relative path.
sed -i.bak 's/\.\./\./g' docs/index.html; rm docs/*.bak
Copy link
Owner

@lukasschwab lukasschwab Jun 5, 2021

Choose a reason for hiding this comment

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

One note: it is nice to have fuzzy search in the sidebar, even for a package this simple. I'll keep an eye on your changelog in case you re-add it!

Using sed to modify the relative path was a hack, of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I see where you are coming from. The intention here was that users are already familar with their browser's builtin search, and not showing the in-page search makes it clear that there is nothing else that can be searched. :) I'll gather a bit more feedback, if it turns out that folks universally still want fuzzy search, we may just re-add it. :)

@lukasschwab lukasschwab merged commit 2c7f4e3 into lukasschwab:master Jun 5, 2021
@mhils
Copy link
Contributor Author

mhils commented Jun 5, 2021

Well-spotted. Maybe it's a mistake in how I designed this package, but pdoc's index wasn't super useful for this project––it just lists one module, arxiv, so I figured I'd pull that module's docs page out to replace the index. Not sure that this is typical, but glad to hear it's better supported now!

pdoc always generates a module index for consistency, but doesn't link to it if only one module is being documented. In your case it detected two modules - arxiv (arxiv/__init__.py) and arxiv.àrxiv (arxiv/arxiv.py) - which is why the link was there.

FWIW you could also simplify your package to a single file (mv ./arxiv/arxiv.py ./arxiv.py; rm -rf ./arxiv), but I didn't want to impose any structural changes to your work here. :)

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.

None yet

2 participants