[Feature] Replace os.system() with subprocess.run() and improve npm installation check #380
Description
Problem
The current script uses os.system() to run commands, which is:
- Unsafe (can lead to shell injection).
- Doesn’t capture errors or output properly.
- Installs github-readme-to-html every time, even if it’s already installed, wasting time and bandwidth.
Description
Replace os.system() with subprocess.run() and implement a check so that github-readme-to-html is installed only if it’s not already installed.
Benefits:
- Safer command execution.
- Proper error handling with exit codes.
- Avoids unnecessary npm installations
Alternatives
Keep using os.system() and reinstall the package every time (current behavior).
The proposed solution is safer, more efficient, and user-friendly.
Additional context
Currently, the script always reinstalls github-readme-to-html and uses os.system(), which is unsafe and doesn’t provide proper error handling.
Using subprocess.run() with a check for existing npm installations will make the script safer, faster, and more user-friendly.
Implementation
Replace all os.system() calls with subprocess.run().
Check if github-readme-to-html is already installed before attempting installation.
Handle errors using specific exceptions like subprocess.CalledProcessError.
Update user messages to clearly indicate progress and errors.
- I would be interested in implementing this feature.