Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-69405: Add check for idle's help.html to CI#143742
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
075a0697dbf673571ff81c8a5cc0471961e0f91167f0b9c5f9d3311645e7f69File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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: | ||
| @@ -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 }}' | ||
| @@ -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 | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer passing | ||
| retention-days: 1 | ||
| # Run "doctest" on HEAD as new syntax doesn't exist in the latest stable release | ||
| doctest: | ||
| 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 | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you suggesting we simply fail if idle.rST is modified? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" | ||
| @@ -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 Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it the case that make can run Python with a file argument but not a MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, make can handle it. | ||
| .PHONY: gettext | ||
| gettext: BUILDER = gettext | ||
| gettext: override SPHINXOPTS := --doctree-dir build/doctrees-gettext $(SPHINXOPTS) | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this stay in Lib/idlelib/help.py? MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would have to build CPython if we want to be able to modify it, that's not particularly fast? Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
| 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() |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.