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

Convert PR numbers in docs/change_log to clickable links #4346

Merged

Conversation

sumezulike
Copy link
Contributor

Description

Closes #4258.

This adds a listener to the sphinx include-read event to find all PR numbers (#X) with regex and replace them with the corresponding link [(#X)](https://github.com/psf/black/pull/X).

I didn't add an entry to CHANGES.md since this seems like a rather minor docs change.

Checklist - did you ...

  • [-] Add an entry in CHANGES.md if necessary?
  • [-] Add / update tests if necessary?
  • [-] Add new / update outdated documentation?

Uses the sphinx include-read event to regex replace all occurrences of a PR number `(#X)` with a link `[(#X)](https://github.com/psf/black/pull/X)`.
@hauntsaninja hauntsaninja added the skip news Pull requests that don't need a changelog entry. label May 4, 2024
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks!

docs/conf.py Outdated


def handle_include_read(
app: Sphinx,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we're not formatting this file with Black? Let's fix that in a separate PR though.

docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
@JelleZijlstra
Copy link
Collaborator

The preview is at https://black--4346.org.readthedocs.build/en/4346/change_log.html, looks great!

@cobaltt7
Copy link
Contributor

cobaltt7 commented May 4, 2024

Looks very clean! I skimmed the changelog and I did find a few inconsistencies that weren't always linked properly:

Add extra blank lines in stubs in a few cases (#3564, #3862)
Upgrade to mypy 1.7.1 (#4049) (#4069)
Revert the change ... (#3940) for better compatibility ...  (#4137)
... This fixes issue #1629 and all of its many many duplicates. (#2126)
added `--extend-exclude` argument (PR #2005)
re-implemented support for explicit trailing commas ... (#1288 and duplicates)

It may be easiest to linkify all mentions, even if outside parenthesis.

@JelleZijlstra JelleZijlstra merged commit f22b243 into psf:main May 4, 2024
16 checks passed
@JelleZijlstra
Copy link
Collaborator

Oops sorry, I merged just as @RedGuy12's comment appeared. Agree it makes sense to linkify all instances of #NNN even if they are not immediately within parentheses.

@sumezulike
Copy link
Contributor Author

Good point, I overlooked that there were inconsistencies, didn't scroll that far 😄
I initially decided to make it strict because the docs dictate the format like

- `Black` is now more awesome (#X)

Also, some people mention issues in the descriptions which would then cause invalid links.
I can make the matching more lenient though, maybe all #numbers inside parentheses?
So Fixes stuff (#123 and #456) would match both but Fixes #123 (#456) only matches 456?

@JelleZijlstra
Copy link
Collaborator

Issue links are actually fine, GitHub redirects them. I put up an implementation in #4347.

@sumezulike
Copy link
Contributor Author

Alright, thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news Pull requests that don't need a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create URL links to actual PRs rather than simply referencing the PR number
4 participants