Skip to content

Conversation

@StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented Jan 12, 2026

I expect it to fail till #143718 is merged.

I had to refactor copy_strip to a separate file to avoid having to build Python.


📚 Documentation preview 📚: https://cpython-previews--143742.org.readthedocs.build/

🏃 Run preview 🏃: https://github.com/python/cpython/actions/runs/20930506392/job/60140508484?pr=143742

@StanFromIreland
Copy link
Member Author

A downside of how I currently have it is that it need the entire docs job to finish (i.e. it includes waiting for the doc tests), we could have it run earlier by moving it under the docs job (though I don't think that is particularly nice), however since it runs in 7 seconds I think it is a negligible issue.

@StanFromIreland
Copy link
Member Author

StanFromIreland commented Jan 12, 2026

@webknjaz could you please take a peek at this?

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Should this new job also be required by "All required checks pass"?

image

https://github.com/python/cpython/actions/runs/20930506392?pr=143742


A downside of how I currently have it is that it need the entire docs job to finish (i.e. it includes waiting for the doc tests), we could have it run earlier by moving it under the docs job (though I don't think that is particularly nice), however since it runs in 7 seconds I think it is a negligible issue.

The time isn't a problem, but there's quite a bit of extra complexity and config to achieve this, when it could be ~10 extra lines to add a couple of steps to the existing docs job.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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
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.

hugovk

This comment was marked as duplicate.

@StanFromIreland
Copy link
Member Author

Should this new job also be required by "All required checks pass"?

Terry asked me to not make it required on the issue.

StanFromIreland and others added 2 commits January 12, 2026 20:46
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

This is getting messy, so lets back up. IDLE's objective is to make an offlline frozen copy of its doc available. At one time, the Windows installer installed propriety .chm files derived from the .html files. IDLE's F1 brought up the python .chm index. I believe that this was true at the time the IDLE doc display, using the help.html copy, was added.

Currently, the Windows installer installs the Doc/build/html tree beside python.exe and F1 displays index.html. Ditto for macOS, though the location is more obscure. In either case, Modules/idlelib displays idle.html. So on Windows and Mac, the local copy is not needed. I don't know what the situation is on Linux and *nix in general, but I have the impression that at least some distributions also include the docs with Python. So maybe we can make this issue moot.

EDIT: I could look for the F1 code to see what it does on *nix, but I need sleep.

Comment on lines +147 to +150
.PHONY: idlehelp
idlehelp: build/html/library/idle.html
$(PYTHON) ../Tools/build/generate_idle_help.py

Copy link
Member

Choose a reason for hiding this comment

The 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 -c "code" argument?

with:
python-version: '3'
- name: 'Regenerate Lib/idlelib/help.html'
run: make idlehelp
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member

Choose a reason for hiding this comment

The 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
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.

@bedevere-app
Copy link

bedevere-app bot commented Jan 13, 2026

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants