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

put source code below docstring #343

Closed
abubelinha opened this issue Feb 3, 2022 · 15 comments
Closed

put source code below docstring #343

abubelinha opened this issue Feb 3, 2022 · 15 comments

Comments

@abubelinha
Copy link

abubelinha commented Feb 3, 2022

Problem Description

When browsing documentation, the source code details are hidden and docstring appears in its natural place, right below the function definition ... until you click the "view source" option.
After clicking that option, source code appears between function definition and docstring ... so docstring gets displaced down many lines (as many as the code length).
So all the rich docstring format and all links it might contain disappear from our scope (until we close the source code).

  1. Default view:
<div class="attr function">...</div>     <details> <summary>View Source</summary>
<div class="codehilite"></div></details>   <---- HIDDEN STUFF
<div class="docstring">...</div>   <----   possible links to other modules and functions
  1. Click "view source"
<div class="attr function">...</div>     <details> <summary>View Source</summary>
    <div class="codehilite">
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    </div>
</details>
<div class="docstring">...</div>   <----   Hey!!!  I was at top but I am here now. Move up & down to read me if you want.

Proposal

In my opinion, it might be much better to keep docstring below function definition, and put source code below docstring.
So we can still see the nice docstring in its original place, and use the links to other functions and modules without having to move down.

<div class="attr function">...</div>
<div class="docstring">...</div>   <----   Don't worry. I am still here and will always be.
<details>
    <summary>View Source</summary>
    <div class="codehilite">
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    ...
    </div>
</details>

Alternatives

Can I configure this using templates?
Anyway, I think the default look would improve with this change.

@mhils
Copy link
Member

mhils commented Feb 3, 2022

In my opinion, it might be much better to keep docstring below function definition, and put source code below docstring.
So we can still see the nice docstring in its original place, and use the links to other functions and modules without having to move down.

I think no location is inherently better, I personally prefer to have it right at the top. There's a practical argument that putting it after the docstring is more tricky as it would require quite a bit of additional whitespace to make sure it doesn't overlay the docstring.

You can of course change it in a custom template by swapping the order in the members macro.

@mhils mhils closed this as completed Feb 3, 2022
@abubelinha
Copy link
Author

abubelinha commented Feb 4, 2022

Thanks. I don't really know much about these templates.

You mean changing from this

    {% if doc.type != "variable" %}
        {{ view_source(doc) }}
    {% endif %}
    {{ docstring(doc) }}

to this?

    {{ docstring(doc) }}
    {% if doc.type != "variable" %}
        {{ view_source(doc) }}
    {% endif %}

@mhils
Copy link
Member

mhils commented Feb 4, 2022

Yes, exactly. You can find some more resources on how templates work at
https://pdoc.dev/docs/pdoc.html#editing-pdocs-html-template
and https://github.com/mitmproxy/pdoc/tree/main/examples/custom-template.

@abubelinha
Copy link
Author

Great.
So I'll have to keep an eye on what I changed, or it will get overwritten when I upgrade pdoc version.
For example, I guess line numbers issue #328 could affect next release templates?

Or perhaps pdoc has a more sophisticated way of overcoming this, like using templates stored in a non-standard path?

@mhils
Copy link
Member

mhils commented Feb 4, 2022

You can overwrite only specific parts of a template with a local file. Pleae read the links I posted. :)

@abubelinha
Copy link
Author

abubelinha commented Feb 4, 2022

Yes sorry ... I had seen that and was just about to edit my comment when you answered.
A couple of questions, just to be sure I am understanding more or less correctly the examples:

1. When you say "overwrite specific parts of a template" (using the {% extends "default/index.html.jinja2" %} syntax):

I understand I must rewrite full "named" sections, like ...
{% block nav_title %} ... {% endblock %}
or perhaps (not sure about this one) ...
{% defaultmacro member(doc) %} ... {% enddefaultmacro %}
Correct?

So if I just want to add a line somewhere within the body block, I need to locate the "subblock" where it is ...

{% if module.members %}
    <h2>API Documentation</h2>
    <h4>{{ module.fullname }}</h4>  {# <-------------       MY ADDITION        #}
    {{ nav_members(module.members.values()) }}
{% endif %}

I guess the {% if %} ... {% endif %} "part" is not a "replaceable block" on its own, because there could be many if-endif subsections like that in the same template. So I would rather need to include the FULL smallest real "block" container section, which would be {% block nav %} ... {% endblock %} in my custom template. Not sure if this only applies to "block" sections, or we can overwrite something else.
Correct?

2) Does that mean we cannot add sections which do not exist using the {% extends "default/index.html.jinja2" %} syntax?

For example, this {% block search_js %} ... {% endblock %} you added at the very end of index.html.jinja2 one month ago ... :

{% block search_js %}
    {% if search %}
        {% include "search.html.jinja2" %}
    {% endif %}
{% endblock %}

I wouldn't be able to add something like that using just a {% extends "default/index.html.jinja2" %} trick.

But I should instead put a FULL custom index.html.jinja2 in my template_directory, so it replaces the default/ templates directory. Correct?

Thanks !

@mhils
Copy link
Member

mhils commented Feb 4, 2022

  1. yes, all correct.

  2. You cannot add new blocks, but you can add content to an existing block. See https://github.com/mitmproxy/pdoc/blob/main/examples/custom-template/module.html.jinja2 for an example and
    https://jinja.palletsprojects.com/en/3.0.x/templates/#child-template for a detailed description.

@abubelinha
Copy link
Author

Awesome. Thank you so much. I have already moved my docstrings above the source code section and all looks great.
I am just wondering what you meant here:

There's a practical argument that putting it after the docstring is more tricky as it would require quite a bit of additional whitespace to make sure it doesn't overlay the docstring.

I don't see anything odd.
Perhaps that could happen if the very last line of a docstring is long enough to reach exactly the top right of its section, where the <summary>View Source</summary> appears? (i.e. a long string without break spaces, like a long url or so).

So I'd rather do this instead of what I said above?

    {{ docstring(doc) }}  {# I already moved this up #}
    {% if doc.type != "variable" %}
        <br>  {# I should also add this just to be safe? #}
        {{ view_source(doc) }}
    {% endif %} 

@mhils
Copy link
Member

mhils commented Feb 5, 2022

Yes, a long last line is why it's not enabled in pdoc by default.

@abubelinha
Copy link
Author

I see. I just added a long non-spaced line with Chrome DevTools and it actually gets too close, but they don't overlay. That looks more than OK for my needs.

BTW, I think there are CSS ways of avoiding the long line overflow the right div border (so it splits and continues in the next line).
Not sure though ... I have very poor CSS skills.
That is pretty related to this comment about the line numbers issue.
Not sure if you missed it, but I would like to know if you have idea of how to implement that (i.e. in my template_directory, unless you have already solved it in the pending line numbers new release).

Thanks a lot anyway.

@mhils
Copy link
Member

mhils commented Feb 5, 2022

I'm sure you can tweak line wrapping with CSS, but I don't have the capacity or interest to look into that. Sorry.

@abubelinha
Copy link
Author

abubelinha commented Feb 5, 2022

OK. But when I mentioned that in #328 I was not meaning long non-spaced lines.

In current pdoc documentation view-source, there are many examples where source code is right-cutted even for spaced lines.
It's a very serious legibility issue, IMHO.
I have to go down to right-move the scroll bar at bottom of source code, and then go up again to read the cutted line, down again to left-move the bar to read the next line begin ... down-right-up - read - down-left-up - read - down-right-up - read - down-left-up - read ... it's a nightmare. That's why I mentioned it.

Might this be an issue in my browser alone?
Chrome Version 96.0.4664.93 (Official Build) (64-bit). Windows 7

Nevermind. I'll see if I can solve this experimenting css try-error with template_directory. I have a question though.

This template example seems the proper way to do it. The comment above block says:
We can extend a block using {{ super() }}, in this case we add custom css at the end of the style block.

... but then inside block there is an additional piece of code: "| safe":
{{ super() | safe }}

What's that | safe for?

@mhils
Copy link
Member

mhils commented Feb 5, 2022

Please check out the Jinja2 documentation for templating. :)

@abubelinha
Copy link
Author

abubelinha commented Feb 5, 2022

I did check this, but I give up. I don't get the idea of what is safe or unsafe in this context.
I guess it has something to do with this error(?) here (huge characters because you forgot to escape <h1> in this function???)
https://pdoc.dev/docs/pdoc/markdown2.html#Markdown.header_id_from_text

Anyway, I'll cross my fingers and let it be "| safe" like in your example, while playing with css. :)

Regarding my css experiments:

  1. I guess this would let long lines wrap (idea taken from here):
.codehilite pre {white-space: break-spaces;}

But I changed my mind after seeing the effect: at least for Python code, it's a bad idea.
Indenting would be kept at line beginning, but not at it's continuation if it wraps because of being too long.
Anyway, I left my CSS here just in case somebody else find that useful.

  1. Another option to avoid right-side cutting effect in pdoc's default templates, is changing the .codehilite pre css code from "overflow: auto;" to some other value. I tried with "initial" but other values do the trick too.
.codehilite pre { overflow: initial; } 

Not sure about what I am doing, so this could have some collateral effects.
But for now, I prefer this to avoid going up & down to use each section bottom overflow-x bar (with this trick, I can use the page body bottom overflow-x bar, which is always visible).

To appreciate its usefulness, you'll have to pdocument a long and wide function (with many too-wide or very-indented lines of code), and see how it looks like with default pdoc css (unreadable right side of text).

Anyway, thanks a lot for guiding me to solve my problems.

@mhils
Copy link
Member

mhils commented Feb 5, 2022

huge characters because you forgot to escape <h1> in this function???

pdoc does not escape HTML tags in docstrings, this is expected behavior.
pdoc.markdown2 is a vendored copy of the markdown2 library, which for practical reasons is included without any modifications (even if it's somewhat ugly).

To appreciate its usefulness, you'll have to pdocument a long and wide function (with many too-wide or very-indented lines of code), and see how it looks like with default pdoc css (unreadable right side of text).

I'm perfectly aware of the current layout. It will stay this way. pdoc displays a bit above 100 characters per line without scrolling. For reference, black has a maximum line length of 88 characters1. It is very easy to scroll horizontally on all modern desktop OS and mobile devices. Yes, this is opinionated, but please accept that I'm not going to discuss it any further.

Footnotes

  1. https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length

@mitmproxy mitmproxy locked as resolved and limited conversation to collaborators Feb 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants