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

doc: extract_content.py not copying images in a table #21466

Closed
dbkinder opened this issue Dec 18, 2019 · 3 comments · Fixed by #21509
Closed

doc: extract_content.py not copying images in a table #21466

dbkinder opened this issue Dec 18, 2019 · 3 comments · Fixed by #21509
Assignees
Labels
area: Documentation bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@dbkinder
Copy link
Contributor

dbkinder commented Dec 18, 2019

Describe the bug
The doc build script extract_content.py is used to scan .rst files looking for referenced files (images and includes for example) and copies them to the proper location in the _build folder.

The script is missing such references if they're within a drawn table (as encountered in PR #21361), and doesn't copy the referenced images, resulting in a doc build error that the images can't be found.

I don't think the script recognized the .. image directive because there's a pipe symbol earlier on the line with the directive, and there are also two image directives on the same line (not properly handled in the script either, I suspect).

    +-------------------------------------+-------------------------------------------+
    | .. image:: ./spidongle_assy6791.png | .. image:: ./spidongle_assy6791_view2.png |
    |      :width: 300px                  |      :width: 380px                        |
    |      :align: center                 |      :align: center                       |
    |      :alt: SPI DONGLE ASSY 6791     |      :alt: SPI DONGLE ASSY 6791 view 2    |
    |                                     | .. image:: ./dediprog_connector_2.png     |
    |                                     |      :width: 380px                        |
    |                                     |      :align: center                       |
    |                                     |      :alt: SPI DONGLE ASSY 6791 Connected |
    |                                     |                                           |
    +-------------------------------------+-------------------------------------------+

Workaround was to use a list-table directive (which did work), but the script should be updated to handle this valid use of images in a table.

@dbkinder dbkinder added the bug The issue is a bug, or the PR is fixing a bug label Dec 18, 2019
@dbkinder
Copy link
Contributor Author

@carlescufi @mbolivar Not a common problem, but it was interesting to diagnose.

@jhedberg jhedberg added the priority: low Low impact/importance bug label Dec 18, 2019
@carlescufi carlescufi assigned ulfalizer and unassigned carlescufi Dec 18, 2019
@carlescufi
Copy link
Member

@dbkinder thanks for the report. I've assigned it to @ulfalizer who has been doing a lot of work lately with the documentation Python scripts.

ulfalizer added a commit to ulfalizer/zephyr that referenced this issue Dec 19, 2019
Allow '.. <figure/include/image/...>:: <path>' to appear anywhere within
a line, and find multiple directives on a single line. This is needed to
find files included e.g. within tables.

Implemented by making the <path> part of the regex more specific and
searching for matches anywhere within the contents of the file. Should
be a bit faster too.

Maybe there's some tiny potential for false positives, but this
generates the same file list as the old version for the current docs at
least.

Fixes: zephyrproject-rtos#21466

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
@ulfalizer
Copy link
Collaborator

Fix: #21509

nashif pushed a commit that referenced this issue Dec 20, 2019
Allow '.. <figure/include/image/...>:: <path>' to appear anywhere within
a line, and find multiple directives on a single line. This is needed to
find files included e.g. within tables.

Implemented by making the <path> part of the regex more specific and
searching for matches anywhere within the contents of the file. Should
be a bit faster too.

Maybe there's some tiny potential for false positives, but this
generates the same file list as the old version for the current docs at
least.

Fixes: #21466

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Documentation bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants