FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-69405: Add check for idle's help.html to CI by StanFromIreland · Pull Request #143742 · python/cpython · GitHub

/ cpython Public
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .py  (3) .rst  (1) .yml  (4) No extension  (2) All 4 file types selected
Only manifest files
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
7 changes: 4 additions & 3 deletions .github/CODEOWNERS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,10 @@ Lib/test/test_html*.py @ezio-melotti
Tools/build/parse_html5_entities.py @ezio-melotti

# IDLE
Doc/library/idle.rst @terryjreedy
Lib/idlelib/ @terryjreedy
Lib/turtledemo/ @terryjreedy
Doc/library/idle.rst @terryjreedy
Lib/idlelib/ @terryjreedy
Lib/turtledemo/ @terryjreedy
Tools/build/generate_idle_help.py @terryjreedy

# importlib.metadata
Doc/library/importlib.metadata.rst @jaraco @warsaw
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/build.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ jobs:
if: fromJSON(needs.build-context.outputs.run-docs)
uses: ./.github/workflows/reusable-docs.yml

check-idle-help-doc:
name: IDLE help doc
needs: [build-context, check-docs]
if: fromJSON(needs.build-context.outputs.run-idle-help-doc)
uses: ./.github/workflows/reusable-idle-help-doc.yml
with:
idle-html-artifact-id: ${{ needs.check-docs.outputs.idle-html-artifact-id }}

check-autoconf-regen:
name: 'Check if Autoconf files are up to date'
# Don't use ubuntu-latest but a specific version to make the job
Expand Down Expand Up @@ -711,6 +719,7 @@ jobs:
build-ubuntu-ssltests-openssl,
test-hypothesis,
cifuzz,
check-idle-help-doc,
allowed-skips: >-
${{ !fromJSON(needs.build-context.outputs.run-docs) && 'check-docs,' || '' }}
${{
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/reusable-context.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ on: # yamllint disable-line rule:truthy
run-docs:
description: Whether to build the docs
value: ${{ jobs.compute-changes.outputs.run-docs }} # bool
run-idle-help-doc:
description: Whether to check the IDLE help doc check
value: ${{ jobs.compute-changes.outputs.run-idle-help-doc }} # bool
run-ios:
description: Whether to run the iOS tests
value: ${{ jobs.compute-changes.outputs.run-ios }} # bool
Expand Down Expand Up @@ -57,6 +60,7 @@ jobs:
run-android: ${{ steps.changes.outputs.run-android }}
run-ci-fuzz: ${{ steps.changes.outputs.run-ci-fuzz }}
run-docs: ${{ steps.changes.outputs.run-docs }}
run-idle-help-doc: ${{ steps.changes.outputs.run-idle-help-doc }}
run-ios: ${{ steps.changes.outputs.run-ios }}
run-macos: ${{ steps.changes.outputs.run-macos }}
run-tests: ${{ steps.changes.outputs.run-tests }}
Expand Down
13 changes: 13 additions & 0 deletions .github/workflows/reusable-docs.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ name: Reusable Docs

on:
workflow_call:
outputs:
idle-html-artifact-id:
description: 'Artifact ID for the built idle.html'
value: ${{ jobs.build-doc.outputs.idle-html-artifact-id }}
workflow_dispatch:

permissions:
Expand All @@ -19,6 +23,8 @@ jobs:
name: 'Docs'
runs-on: ubuntu-latest
timeout-minutes: 60
outputs:
idle-html-artifact-id: ${{ steps.upload-idle-html.outputs.artifact-id }}
env:
branch_base: 'origin/${{ github.event.pull_request.base.ref }}'
branch_pr: 'origin/${{ github.event.pull_request.head.ref }}'
Expand Down Expand Up @@ -75,6 +81,13 @@ jobs:
--fail-if-regression \
--fail-if-improved \
--fail-if-new-news-nit
- name: 'Upload built idle.html'
id: upload-idle-html
uses: actions/upload-artifact@v4
with:
name: idle-html
path: Doc/build/html/library/idle.html
Comment thread
StanFromIreland marked this conversation as resolved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'd prefer passing Doc/build/html/library/ between the jobs too: an artifact is an abstract'ish GHA concept for "some/arbitrary files stored for later use" and the path is where to put it when downloading+uploading. But together, the the pair of artifact ID+path to unpack it, is something that shouldn't be maintained in two disconnected contexts w/ an implicit expectation that somebody will keep them in sync.
Though, this would require another step to store it into an output and reuse here + workflow outputs. We could probably pack both into the same output as a JSON string so they aren't separated in refactorings...

retention-days: 1

# Run "doctest" on HEAD as new syntax doesn't exist in the latest stable release
doctest:
Expand Down
43 changes: 43 additions & 0 deletions .github/workflows/reusable-idle-help-doc.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
name: Reusable check IDLE help

on:
workflow_call:
inputs:
idle-html-artifact-id:
description: 'Artifact ID for the built idle.html'
required: true
type: string

permissions: {}

env:
FORCE_COLOR: 1

jobs:
check-idle-doc-sync:
name: 'Check if IDLE help needs regenerating'
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- uses: actions/checkout@v6
with:
persist-credentials: false
- name: 'Download built idle.html'
uses: actions/download-artifact@v7
with:
artifact-ids: ${{ inputs.idle-html-artifact-id }}
path: Doc/build/html/library
- name: 'Set up Python'
uses: actions/setup-python@v6
with:
python-version: '3'
- name: 'Regenerate Lib/idlelib/help.html'
run: make idlehelp

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It appears that rather than compare last change dates you are making a potentially revised file and looking for a non-empty diff (but not committing). The latter seems much harder. Is yaml unable to do the former?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Are you suggesting we simply fail if idle.rST is modified?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

FTR, if the only check we want here is comparing last edit dates of two files, I'd place it in makefile and would drop that separate step. Though, the semantics of such a check is unclear — the timestamps are likely to differ by seconds unless there's an automation that sets these to the same value. It seems like the only reliable check would be whether their last change happened in the same Git commit, if I understand the implied relation between the two files.

working-directory: Doc
- name: 'Check for changes'
run: |
git diff --exit-code Lib/idlelib/help.html || {
echo "Lib/idlelib/help.html is not up to date."
echo "Run make idlehelp in the Doc directory."
exit 1
}
5 changes: 5 additions & 0 deletions Doc/Makefile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ help:
@echo " coverage to check documentation coverage for library and C API"
@echo " doctest to run doctests in the documentation"
@echo " pydoc-topics to regenerate the pydoc topics file"
@echo " idlehelp to regenerate Lib/idlelib/help.html"
@echo " dist to create a \"dist\" directory with archived docs for download"
@echo " check to run a check for frequent markup errors"

Expand Down Expand Up @@ -143,6 +144,10 @@ pydoc-topics: build
"cp build/pydoc-topics/topics.py ../Lib/pydoc_data/topics.py" \
"&& cp build/pydoc-topics/module_docs.py ../Lib/pydoc_data/module_docs.py"

.PHONY: idlehelp
idlehelp: build/html/library/idle.html
$(PYTHON) ../Tools/build/generate_idle_help.py

Comment on lines +147 to +150

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is it the case that make can run Python with a file argument but not a -c "code" argument?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No, make can handle it.

.PHONY: gettext
gettext: BUILDER = gettext
gettext: override SPHINXOPTS := --doctree-dir build/doctrees-gettext $(SPHINXOPTS)
Expand Down
2 changes: 1 addition & 1 deletion Doc/library/idle.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ IDLE --- Python editor and shell
single: Integrated Development Environment

..
Remember to update Lib/idlelib/help.html with idlelib.help.copy_strip() when modifying this file.
Remember to update Lib/idlelib/help.html with make idlehelp when modifying this file.

--------------

Expand Down
37 changes: 0 additions & 37 deletions Lib/idlelib/help.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -248,43 +248,6 @@ def __init__(self, parent, filename, title):
self.grid_rowconfigure(0, weight=1)


def copy_strip(): # pragma: no cover
"""Copy the text part of idle.html to idlelib/help.html while stripping trailing whitespace.

Files with trailing whitespace cannot be pushed to the git cpython
repository. For 3.x (on Windows), help.html is generated, after
editing idle.rst on the master branch, with
sphinx-build -bhtml . build/html
python_d.exe -c "from idlelib.help import copy_strip; copy_strip()"
Check build/html/library/idle.html, the help.html diff, and the text
displayed by Help => IDLE Help. Add a blurb and create a PR.

It can be worthwhile to occasionally generate help.html without
touching idle.rst. Changes to the master version and to the doc
build system may result in changes that should not change
the displayed text, but might break HelpParser.

As long as master and maintenance versions of idle.rst remain the
same, help.html can be backported. The internal Python version
number is not displayed. If maintenance idle.rst diverges from
the master version, then instead of backporting help.html from
master, repeat the procedure above to generate a maintenance
version.
"""
src = join(abspath(dirname(dirname(dirname(__file__)))),
'Doc', 'build', 'html', 'library', 'idle.html')
dst = join(abspath(dirname(__file__)), 'help.html')

with open(src, 'r', encoding="utf-8") as inn, open(dst, 'w', encoding="utf-8") as out:
copy = False
for line in inn:
if '<section id="idle">' in line: copy = True
if '<div class="clearer">' in line: break
if copy: out.write(line.strip() + '\n')

print(f'{src} copied to {dst}')


def show_idlehelp(parent):
"Create HelpWindow; called from Idle Help event handler."
filename = join(abspath(dirname(__file__)), 'help.html')
Expand Down
26 changes: 15 additions & 11 deletions Tools/build/compute-changes.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import os
import subprocess
from dataclasses import dataclass
from dataclasses import dataclass, fields
from pathlib import Path

TYPE_CHECKING = False
Expand Down Expand Up @@ -56,6 +56,7 @@ class Outputs:
run_android: bool = False
run_ci_fuzz: bool = False
run_docs: bool = False
run_idle_help_doc: bool = False
run_ios: bool = False
run_macos: bool = False
run_tests: bool = False
Expand Down Expand Up @@ -148,6 +149,7 @@ def process_changed_files(changed_files: Set[Path]) -> Outputs:
run_tests = False
run_ci_fuzz = False
run_docs = False
run_idle_help_doc = False
run_windows_tests = False
run_windows_msi = False

Expand All @@ -165,6 +167,8 @@ def process_changed_files(changed_files: Set[Path]) -> Outputs:
has_platform_specific_change = False
if file.name == "reusable-docs.yml":
run_docs = True
if file.name == "reusable-idle-help-doc.yml":
run_idle_help_doc = True
if file.name == "reusable-windows-msi.yml":
run_windows_msi = True
if file.name == "reusable-macos.yml":
Expand Down Expand Up @@ -201,6 +205,11 @@ def process_changed_files(changed_files: Set[Path]) -> Outputs:
if doc_file:
run_docs = True

# Check for changed IDLE docs
if file in (Path("Doc/library/idle.rst"),
Path("Tools/build/generate_idle_help.py")):
run_idle_help_doc = True

# Check for changed MSI installer-related files
if file.parts[:2] == ("Tools", "msi"):
run_windows_msi = True
Expand Down Expand Up @@ -230,6 +239,7 @@ def process_changed_files(changed_files: Set[Path]) -> Outputs:
run_android=run_android,
run_ci_fuzz=run_ci_fuzz,
run_docs=run_docs,
run_idle_help_doc=run_idle_help_doc,
run_ios=run_ios,
run_macos=run_macos,
run_tests=run_tests,
Expand Down Expand Up @@ -263,16 +273,10 @@ def write_github_output(outputs: Outputs) -> None:
return

with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
f.write(f"run-android={bool_lower(outputs.run_android)}\n")
f.write(f"run-ci-fuzz={bool_lower(outputs.run_ci_fuzz)}\n")
f.write(f"run-docs={bool_lower(outputs.run_docs)}\n")
f.write(f"run-ios={bool_lower(outputs.run_ios)}\n")
f.write(f"run-macos={bool_lower(outputs.run_macos)}\n")
f.write(f"run-tests={bool_lower(outputs.run_tests)}\n")
f.write(f"run-ubuntu={bool_lower(outputs.run_ubuntu)}\n")
f.write(f"run-wasi={bool_lower(outputs.run_wasi)}\n")
f.write(f"run-windows-msi={bool_lower(outputs.run_windows_msi)}\n")
f.write(f"run-windows-tests={bool_lower(outputs.run_windows_tests)}\n")
for field in fields(outputs):
name = field.name.replace("_", "-")
val = bool_lower(getattr(outputs, field.name))
f.write(f"{name}={val}\n")


def bool_lower(value: bool, /) -> str:
Expand Down
46 changes: 46 additions & 0 deletions Tools/build/generate_idle_help.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Could this stay in Lib/idlelib/help.py?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We would have to build CPython if we want to be able to modify it, that's not particularly fast?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't understand the purported need to move copy_strip (and fairly strongly prefer not to). What is 'it'? I cannot imagine a need to modify the code just to check the file modification state.

I am not sure about adding a new make target. Currently, I update help.html (either in a .bat or manually) with
python -c "from idlelib.help import copy_strip; copy_strip()" in the repository, so that 'python' is the repository build and the update happens in the repository. I don't know if there is the equivalent for non-Windows, but copy pasting this command into command prompt is easier than changing directories and invoking make. I would personally prefer a reminder to run the above than to find make.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If we don’t want to move, currently it is useless where it is with a system python, as the paths are relative to the location of the installed idlelib, and the Doc folder is not in /usr/lib/ or somewhere like that.

We could update the function, but then we would have to wait for the next 3.14 release (and then for actions to pick it up) before we could it.

The other option is building CPython, but that would just make the check over 50x times slower.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"""Copy the text part of idle.html to idlelib/help.html while stripping trailing whitespace.

Files with trailing whitespace cannot be pushed to the git cpython
repository. help.html is regenerated, after
editing idle.rst on the master branch, with, in the Doc directory
make idlehelp
Check build/html/library/idle.html, the help.html diff, and the text
displayed by Help => IDLE Help. Add a blurb and create a PR.

It can be worthwhile to occasionally generate help.html without
touching idle.rst. Changes to the master version and to the doc
build system may result in changes that should not change
the displayed text, but might break HelpParser.

As long as master and maintenance versions of idle.rst remain the
same, help.html can be backported. The internal Python version
number is not displayed. If maintenance idle.rst diverges from
the master version, then instead of backporting help.html from
master, repeat the procedure above to generate a maintenance
version.
"""

from os.path import abspath, dirname, join


def copy_strip():
src = join(abspath(dirname(dirname(dirname(__file__)))),
'Doc', 'build', 'html', 'library', 'idle.html')
dst = join(abspath(dirname(dirname(dirname(__file__)))),
'Lib', 'idlelib', 'help.html')

with open(src, encoding="utf-8") as inn, open(dst, 'w', encoding="utf-8") as out:
copy = False
for line in inn:
if '<div class="clearer">' in line:
break
if '<section id="idle">' in line:
copy = True
if copy:
out.write(line.strip() + '\n')

print(f'{src} copied to {dst}')


if __name__ == '__main__':
copy_strip()
Loading

Back | FazBrowse Home | New Git URL