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: Exclude documentation_options.js from default theme #61044

Merged
merged 2 commits into from Dec 10, 2023

Conversation

bdrung
Copy link
Contributor

@bdrung bdrung commented Oct 13, 2021

documentation_options.js from the default theme sets the option URL_ROOT to:

document.getElementById("documentation_options").getAttribute('data-url_root')

This requires that the script element for documentation_options.js includes the tag id="documentation_options" and sets the
data-url_root tag. Otherwise evaluating URL_ROOT will fail and building the documentation during the Debian package build will fail:

dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT

The variable DOCUMENTATION_OPTIONS is directly set layout.html and therefore documentation_options.js does not need to be included. So just exclude it.

@bdrung bdrung requested a review from a team as a code owner October 13, 2021 15:44
@bdrung bdrung requested review from dwoz and removed request for a team October 13, 2021 15:44
bdrung added a commit to bdrung/salt that referenced this pull request Oct 13, 2021
`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Forwarded: saltstack#61044
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
bdrung added a commit to bdrung/salt that referenced this pull request Oct 14, 2021
`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Forwarded: saltstack#61044
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
@bdrung
Copy link
Contributor Author

bdrung commented Dec 11, 2021

Rebased on master.

@bdrung bdrung force-pushed the fix-doc-url-root branch 2 times, most recently from 812755f to 2764359 Compare December 17, 2021 08:48
@bdrung bdrung force-pushed the fix-doc-url-root branch 2 times, most recently from 795d70c to 2d5e050 Compare February 11, 2022 01:55
@bdrung
Copy link
Contributor Author

bdrung commented Feb 11, 2022

Rebased on master. Let's see if all tests passes or if unrelated test cases fail.

bdrung added a commit to bdrung/salt that referenced this pull request Apr 15, 2022
`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Forwarded: saltstack#61044
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
Gbp-Pq: Name doc-Exclude-documentation_options.js-from-default-th.patch
bdrung added a commit to bdrung/salt that referenced this pull request Apr 15, 2022
`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Forwarded: saltstack#61044
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
Gbp-Pq: Name doc-Exclude-documentation_options.js-from-default-th.patch
bdrung added a commit to bdrung/salt that referenced this pull request Apr 16, 2022
`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Forwarded: saltstack#61044
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
Gbp-Pq: Name doc-Exclude-documentation_options.js-from-default-th.patch
@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 17, 2022

@barbaricyawps can you review here please

@Ch3LL Ch3LL added the Sulfur v3006.0 release code name and version label Oct 17, 2022
Copy link
Contributor

@barbaricyawps barbaricyawps left a comment

Choose a reason for hiding this comment

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

Recommend closing this PR as the fix appears to have been merged in by someone else, possibly later.

Copy link
Contributor

@barbaricyawps barbaricyawps left a comment

Choose a reason for hiding this comment

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

@bdrung , I'm afraid I don't understand enough about javascript to understand why this PR is necessary and your description doesn't seem to help me. One thing that I'm confused about is why you're adding a reference to a file named _static/documentation_options.js when this file doesn't exist in the root folder being referenced.

What does setting the js_blacklist do?

`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
@bdrung
Copy link
Contributor Author

bdrung commented Oct 24, 2022

js_blacklist is used in layout.html:

    {% if scriptfile not in js_blacklist %}
    <script type="text/javascript" src="{{ pathto(scriptfile, 1) }}"></script>
    {% endif %}

_static/documentation_options.js is generated by sphinx, but this file is not needed since layout.html specifies everything needed itself:

    <script>
        var DOCUMENTATION_OPTIONS = {
            URL_ROOT:    '{{ url_root }}',
            VERSION:     '{{ release|e }}',
            LANGUAGE: '{{ language }}',
            COLLAPSE_INDEX: false,
            BUILDER: '{{ builder }}',
            FILE_SUFFIX: '{{ '' if no_search_suffix else file_suffix }}',
            LINK_SUFFIX: '{{ link_suffix }}',
            HAS_SOURCE: {{ has_source|lower }},
            SOURCELINK_SUFFIX: '{{ sourcelink_suffix }}',
            NAVIGATION_WITH_KEYS: {{ 'true' if theme_navigation_with_keys|tobool else 'false'}}
        };
    </script>

@MKLeb
Copy link
Contributor

MKLeb commented Oct 27, 2022

@bdrung Would you be able to provide an example of this causing the failure you described? Logs, screenshots, whatever works.

bdrung added a commit to bdrung/salt that referenced this pull request Nov 30, 2022
`documentation_options.js` from the default theme sets the option
`URL_ROOT` to:

```
document.getElementById("documentation_options").getAttribute('data-url_root')
```

This requires that the script element for `documentation_options.js`
includes the tag `id="documentation_options"` and sets the
`data-url_root` tag. Otherwise evaluating `URL_ROOT` will fail and
building the documentation during the Debian package build will fail:

```
dh_sphinxdoc: error: DOCUMENTATION_OPTIONS does not define URL_ROOT
```

The variable `DOCUMENTATION_OPTIONS` is directly set `layout.html` and
therefore `documentation_options.js` does not need to be included. So
just exclude it.

Forwarded: saltstack#61044
Signed-off-by: Benjamin Drung <benjamin.drung@ionos.com>
Gbp-Pq: Name doc-Exclude-documentation_options.js-from-default-th.patch
@Ch3LL
Copy link
Contributor

Ch3LL commented Dec 5, 2022

@bdrung can you answer @MKLeb 's question. Would like to progress this issue.

@Ch3LL Ch3LL added Chlorine v3007.0 and removed Sulfur v3006.0 release code name and version labels Dec 6, 2022
@dwoz dwoz merged commit b9b0c24 into saltstack:master Dec 10, 2023
@bdrung bdrung deleted the fix-doc-url-root branch December 11, 2023 10:23
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

8 participants