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

Change how we embed docsite links in help text. #12261

Merged
merged 2 commits into from
Jul 2, 2021

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Jun 30, 2021

The issue is that we needed to generate URLs that work when displayed in
help messages on the console, but that also cross-link correctly when those same
help messages are used to generate docsite reference pages.

Previously we used raw links with the doc version embedded in the URL (e.g.,
https://www.pantsbuild.org/v2.6/docs/protobuf). And in many cases we had
to enclose that URL in parentheses to avoid issues with readme.com's linkifier
(such as it including a trailing period in the URL).

However it turns out that readme.com now modifies such URLs in markdown
to add the version. So https://www.pantsbuild.org/v2.6/docs/protobuf turns into
https://www.pantsbuild.org/v2.6/v2.6/docs/protobuf which is a bad link.

This change makes all this much more robust by moving the logic from help string
authoring time to docsite page generation time. When authoring help strings you simply
write URLs (either raw or, preferably, using the doc_url() util function). The code that
renders docsite pages detects docsite URLs in strings and rewrites them to proper
intra-doc markdown ([title](doc:slug)). It does so by fetching the titles from the live
pages.

This does assume that any links in generated docs are to files that already exist on the docsite.
But this is true today and is likely to continue to be true (since we have to know the page slug anyway).

[ci skip-build-wheels]

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jun 30, 2021

Note: tested locally and generated the v2.6 docs with this. See nice link at the top of https://www.pantsbuild.org/v2.6/docs/reference-python-protobuf for example.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jun 30, 2021

E.g.,

Screen Shot 2021-06-30 at 11 56 23 AM

vs

Screen Shot 2021-06-30 at 11 56 34 AM

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jun 30, 2021

The interesting changes are in build-support/bin/generate_docs.py and src/python/pants/util/docutil.py (and their tests). The rest is mostly the corresponding boilerplate changes.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jun 30, 2021

h/t to @alexey-tereshenkov-oxb for noticing the problem. This should comprehensively fix it across the site.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Good idea! Thank you Benjy.

Could you please cherry-pick to 2.6? Should this be to 2.5 as well?

@benjyw benjyw merged commit f9b5815 into pantsbuild:main Jul 2, 2021
@benjyw benjyw deleted the fix_docs_urls branch July 2, 2021 02:25
benjyw added a commit to benjyw/pants that referenced this pull request Jul 2, 2021
The issue is that we needed to generate URLs that work when displayed in
help messages on the console, but that also cross-link correctly when those same
help messages are used to generate docsite reference pages.

Previously we used raw links with the doc version embedded in the URL (e.g.,
https://www.pantsbuild.org/v2.6/docs/protobuf). And in many cases we had
to enclose that URL in parentheses to avoid issues with readme.com's linkifier
(such as it including a trailing period in the URL).

However it turns out that readme.com now modifies such URLs in markdown
to add the version. So https://www.pantsbuild.org/v2.6/docs/protobuf turns into
https://www.pantsbuild.org/v2.6/v2.6/docs/protobuf which is a bad link.

This change makes all this much more robust by moving the logic from help string
authoring time to docsite page generation time. When authoring help strings you simply
write URLs (either raw or, preferably, using the doc_url() util function). The code that
renders docsite pages detects docsite URLs in strings and rewrites them to proper
intra-doc markdown ([title](doc:slug)). It does so by fetching the titles from the live
pages.

This does assume that any links in generated docs are to files that already exist on the docsite.
But this is true today and is likely to continue to be true (since we have to know the page slug anyway).

[ci skip-build-wheels]
@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jul 2, 2021

Cherry-picked to 2.6. It doesn't apply cleanly on 2.5, so I'd say no to that.

benjyw added a commit that referenced this pull request Jul 2, 2021
illicitonion added a commit to illicitonion/pants that referenced this pull request Jul 9, 2021
Internal changes omitted from the release notes:

* [internal] `./pants lock` uses transitive closure as inputs ([pantsbuild#12312](pantsbuild#12312))

* [internal] Add experimental `./pants lock` to generate a lockfile with pip-compile ([pantsbuild#12300](pantsbuild#12300))

* upgrade tokio and futures crates ([pantsbuild#12308](pantsbuild#12308))

* upgrade tonic, tower, and hyper crates ([pantsbuild#12294](pantsbuild#12294))

* Set up pants to use the latest version of humbug ([pantsbuild#12302](pantsbuild#12302))

* Refactor BUILD file parsing. ([pantsbuild#12279](pantsbuild#12279))

* Use get_type_hints() to get typed type hints. ([pantsbuild#12287](pantsbuild#12287))

* [internal] Fix parameters of convert_source_to_sources_test ([pantsbuild#12274](pantsbuild#12274))

* Fix assert arguments in a test ([pantsbuild#12273](pantsbuild#12273))

* Introduce a native pants client. ([pantsbuild#11922](pantsbuild#11922))

* Don't run CI in forks ([pantsbuild#12267](pantsbuild#12267))

* Change how we embed docsite links in help text. ([pantsbuild#12261](pantsbuild#12261))
illicitonion added a commit to illicitonion/pants that referenced this pull request Jul 9, 2021
Internal changes omitted from the release notes:

* [internal] `./pants lock` uses transitive closure as inputs ([pantsbuild#12312](pantsbuild#12312))

* [internal] Add experimental `./pants lock` to generate a lockfile with pip-compile ([pantsbuild#12300](pantsbuild#12300))

* upgrade tokio and futures crates ([pantsbuild#12308](pantsbuild#12308))

* upgrade tonic, tower, and hyper crates ([pantsbuild#12294](pantsbuild#12294))

* Set up pants to use the latest version of humbug ([pantsbuild#12302](pantsbuild#12302))

* Refactor BUILD file parsing. ([pantsbuild#12279](pantsbuild#12279))

* Use get_type_hints() to get typed type hints. ([pantsbuild#12287](pantsbuild#12287))

* [internal] Fix parameters of convert_source_to_sources_test ([pantsbuild#12274](pantsbuild#12274))

* Fix assert arguments in a test ([pantsbuild#12273](pantsbuild#12273))

* Introduce a native pants client. ([pantsbuild#11922](pantsbuild#11922))

* Don't run CI in forks ([pantsbuild#12267](pantsbuild#12267))

* Change how we embed docsite links in help text. ([pantsbuild#12261](pantsbuild#12261))
illicitonion added a commit to illicitonion/pants that referenced this pull request Jul 9, 2021
Internal changes omitted from the release notes:

* [internal] `./pants lock` uses transitive closure as inputs ([pantsbuild#12312](pantsbuild#12312))

* [internal] Add experimental `./pants lock` to generate a lockfile with pip-compile ([pantsbuild#12300](pantsbuild#12300))

* upgrade tokio and futures crates ([pantsbuild#12308](pantsbuild#12308))

* upgrade tonic, tower, and hyper crates ([pantsbuild#12294](pantsbuild#12294))

* Set up pants to use the latest version of humbug ([pantsbuild#12302](pantsbuild#12302))

* Refactor BUILD file parsing. ([pantsbuild#12279](pantsbuild#12279))

* Use get_type_hints() to get typed type hints. ([pantsbuild#12287](pantsbuild#12287))

* [internal] Fix parameters of convert_source_to_sources_test ([pantsbuild#12274](pantsbuild#12274))

* Fix assert arguments in a test ([pantsbuild#12273](pantsbuild#12273))

* Introduce a native pants client. ([pantsbuild#11922](pantsbuild#11922))

* Don't run CI in forks ([pantsbuild#12267](pantsbuild#12267))

* Change how we embed docsite links in help text. ([pantsbuild#12261](pantsbuild#12261))

* Add JavacSubsystem and JavacBinary, allowing user-configurable JVM selection ([pantsbuild#12283](pantsbuild#12283))

* Revert "Deprecate unused `--process-execution-local-enable-nailgun` (pantsbuild#11600)" ([pantsbuild#12257](pantsbuild#12257))
illicitonion added a commit that referenced this pull request Jul 9, 2021
Internal changes omitted from the release notes:

* [internal] `./pants lock` uses transitive closure as inputs ([#12312](#12312))

* [internal] Add experimental `./pants lock` to generate a lockfile with pip-compile ([#12300](#12300))

* upgrade tokio and futures crates ([#12308](#12308))

* upgrade tonic, tower, and hyper crates ([#12294](#12294))

* Set up pants to use the latest version of humbug ([#12302](#12302))

* Refactor BUILD file parsing. ([#12279](#12279))

* Use get_type_hints() to get typed type hints. ([#12287](#12287))

* [internal] Fix parameters of convert_source_to_sources_test ([#12274](#12274))

* Fix assert arguments in a test ([#12273](#12273))

* Introduce a native pants client. ([#11922](#11922))

* Don't run CI in forks ([#12267](#12267))

* Change how we embed docsite links in help text. ([#12261](#12261))

* Add JavacSubsystem and JavacBinary, allowing user-configurable JVM selection ([#12283](#12283))

* Revert "Deprecate unused `--process-execution-local-enable-nailgun` (#11600)" ([#12257](#12257))
Eric-Arellano added a commit that referenced this pull request Jul 14, 2021
The code from #12261 is only used in `generate_docs.py`, but was defind in `docutil.py` so it gets bundled into pantsbuild.pants. 

By moving out of `docutil.py`, we remove Pants's dependency on `requests`.

[ci skip-rust]
[ci skip-build-wheels]
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this pull request Jul 15, 2021
The code from pantsbuild#12261 is only used in `generate_docs.py`, but was defind in `docutil.py` so it gets bundled into pantsbuild.pants. 

By moving out of `docutil.py`, we remove Pants's dependency on `requests`.

[ci skip-rust]
[ci skip-build-wheels]
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