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

Support nested pages (python.py) #163

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

sizmailov
Copy link
Contributor

@sizmailov sizmailov commented Jun 27, 2020

This PR adds basic support for nested pages for python.py.

Changes:

  • default_url_formatter-generated .html tree structure mirrors input .rst layout
  • appropriate ../* prefix prepended to references and image sources in nested pages (both .rst-generated code and in templates)
  • enable breadcrumb generation for nested pages

I use relative paths because it would make possible to place C++ and Python API on same domain.

TODO:

  • Show nested structure in pages
  • Copy images with relative paths to avoid clashes
  • Avoid clearing jinja cache
  • Search results
  • Other stuff I forget about or didn't know at first place

@codecov
Copy link

codecov bot commented Jun 27, 2020

Codecov Report

Merging #163 into master will decrease coverage by 0.02%.
The diff coverage is 97.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #163      +/-   ##
==========================================
- Coverage   98.25%   98.22%   -0.03%     
==========================================
  Files          27       27              
  Lines        6647     6716      +69     
  Branches       44       44              
==========================================
+ Hits         6531     6597      +66     
- Misses        116      119       +3     
Impacted Files Coverage Δ
documentation/python.py 99.06% <97.27%> (-0.17%) ⬇️
documentation/_search.py 98.78% <100.00%> (ø)
documentation/search.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6eff54...3861776. Read the comment docs.

@@ -264,7 +264,7 @@ def serialize(self, merge_prefixes=True) -> bytearray:
if e.flags & ResultFlag.HAS_SUFFIX:
output += self.suffix_length_struct.pack(e.suffix_length)
output += e.name.encode('utf-8')
if e.url:
if e.name and e.url:
Copy link
Contributor Author

@sizmailov sizmailov Jun 28, 2020

Choose a reason for hiding this comment

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

This change made to match above lines (250-251):

            if e.name and e.url:
                 offset += len(e.url.encode('utf-8')) + 1

Could be it should be turned around.

_url_formatter = None
_external_data = set()
# A helper function to work-around for stateless Docutils transforms
def get_image_extracter(source, external_data:set, config):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every function call generates new class with captured function parameters, a work-around stateless transform limitation.
Not perfect, but probably not a big overhead in python realm.

I've mocked this usage pattern to measure memory footprint. It turned out that python is able to garbage collect most of unused classes as long as they are not referenced from elsewhere.

Here is test and measurement result: https://gist.github.com/sizmailov/93cb521557dd64019598554626defe23
Note: rightmost column can be turned to zeros via extra gc.collect() call

image_abs_path = os.path.realpath(os.path.join(os.path.dirname(source), image['uri']))
# Use relative path if image within INPUT directory, otherwise absolute
if image_abs_path.startswith(input_path):
image_path = os.path.relpath(image_abs_path, input_path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This always returns relative path to INPUT directory, will be patched later by PatchReferences

@sizmailov sizmailov marked this pull request as ready for review June 28, 2020 11:39
@mosra mosra added the laterz Will get resolved eventually, but not now. label Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
laterz Will get resolved eventually, but not now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants