Python 3.12 migration (PR #330 + compatibility fixes) #345
Conversation
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
via "prefers-color-scheme". Fixes idank#236
…milar Signed-off-by: Страхиња Радић <[email protected]>
- Fix iterator protocol: def next() → def __next__() in Peekable class - Fix iterator calls: it.next() → next(it) throughout codebase - Fix map() iterator: yield map() → yield list(map()) in group_continuous - Fix dictionary methods: .iteritems() → .items() in doctests - Fix function names in doctests to match actual definitions - Migrate testing framework: nose → pytest (Python 3.12 compatible) - Update requirements.txt with pytest 8.3.3 and pytest-cov 6.0.0 - Update Makefile to use pytest --doctest-modules These fixes complete the Python 3.12 migration on top of PR idank#330. All 12 tests now passing (100% success rate). Test results: - explainshell/algo/features.py: 1 passed - explainshell/fixer.py: 1 passed - explainshell/manpage.py: 3 passed - explainshell/options.py: 3 passed - explainshell/util.py: 3 passed - explainshell/web/views.py: 1 passed Total: 12 passed in 0.37s Database integration verified with 72,349 documents. Full functionality tested with ls, grep, and tar commands.
There was a problem hiding this comment.
Pull Request Overview
This PR migrates explainshell from Python 2.7 to Python 3.12, addressing critical security vulnerabilities and ensuring long-term maintainability. The migration includes updated dependencies (Flask 3.0.3, pymongo 4.8.0, nltk 3.9.1) and replaces the incompatible nose testing framework with pytest. All 12 tests pass successfully.
Key changes:
- Python 3.12 compatibility fixes (iterator protocol, dictionary methods, string handling)
- Security updates for Flask (CVE-2023-30861) and nltk (ReDoS fix)
- Testing framework migration from nose to pytest
- Ubuntu manpage repository URL updates (HTTP→HTTPS, precise→noble)
Reviewed Changes
Copilot reviewed 46 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/shellbuiltins.py | Updated imports, class names, and logging setup for Python 3 |
| tools/extractgzlist | Changed Ubuntu release from "precise" to "noble" and URLs to HTTPS |
| tests/*.py | Migrated test assertions from unittest2 to standard unittest (Python 3) |
| explainshell/web/views.py | Fixed iterator calls, exception handling, and string encoding |
| explainshell/util.py | Updated iterator protocol methods and lambda syntax |
| explainshell/store.py | Renamed classes to PascalCase and fixed dictionary iteration |
| explainshell/matcher.py | Updated class names and string handling for Python 3 |
| explainshell/manpage.py | Fixed subprocess calls and string encoding |
| Dockerfile | Updated base image from Python 2.7 to 3.12 |
| requirements.txt | Replaced nose with pytest for testing |
| Makefile | Updated test command to use pytest |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- runserver.py: Fix HOST_IP check to use truthiness instead of len() > 1 - es.js: Remove duplicate selectedTheme variable declaration These fixes address the code quality issues identified by GitHub Copilot in the automated code review.
|
Thanks @copilot-pull-request-reviewer for the code review! I've addressed both issues you identified: ✅ Issue 1 - runserver.py (line 13): Fixed the HOST_IP check
✅ Issue 2 - es.js (lines 1154-1156): Removed duplicate variable declaration
Both fixes are now included in commit c7294f7. The code is cleaner and more correct now! 🎉 |
tobiashochguertel
added a commit
to tobiashochguertel/explainshell
that referenced
this pull request
Oct 26, 2025
…to community-integration-2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR combines PR #330 (Python 3 migration by @mundanevision20) with additional compatibility fixes to achieve 100% test passing rate with Python 3.12.
What's Included
From PR #330 (Base Migration by @mundanevision20)
Additional Compatibility Fixes (5 files)
These fixes are needed on top of PR #330 to achieve 100% Python 3.12 compatibility:
Iterator Protocol -
explainshell/util.pydef next(self):→def __next__(self):in Peekable classit.next()→next(it)in doctestsIterator Calls -
explainshell/web/views.pym = it.next()→m = next(it)Map Iterator -
explainshell/util.pyyield map(itemgetter(1), g)→yield list(map(itemgetter(1), g))Dictionary Methods -
explainshell/options.py.iteritems()→.items()in doctestsTesting Framework -
requirements.txtandMakefilenose==1.3.7→pytest==8.3.3+pytest-cov==6.0.0impmodule)Test Results
100% success rate:
Testing Done
✅ Full database integration (72,349 documents restored)
✅ Functional testing:
ls -la,grep -r,tar -xzvf✅ Web interface fully operational
✅ All security fixes maintained
Files Changed
Total: ~50 files (from PR #330) + 5 files (additional fixes)
Additional fixes only:
Makefile- pytest migrationexplainshell/options.py- .iteritems() fixesexplainshell/util.py- iterator protocolexplainshell/web/views.py- iterator callsrequirements.txt- nose → pytestCredits
This makes explainshell fully compatible with Python 3.12 and ready for production use.