-
Notifications
You must be signed in to change notification settings - Fork 17
Add 'first run' configuration experience to prompt users to modify system settings #98
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a first-run configuration experience that prompts users to modify system settings and improves validation of environment requirements. Key changes include a new test suite for first-run checks, native and Python module enhancements for system configuration, and updates to command handling for initiating first-run flows.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_firstrun.py | New tests for first-run configuration and alias checks |
| tests/conftest.py | Added log capture functions to support new test assertions |
| src/pymanager/main.cpp | Introduced a new function to configure long paths via the registry |
| src/manage/firstrun.py | Implemented first-run configuration logic and various system checks |
| src/manage/commands.py | Integrated new first-run and configuration commands into the CLI interface |
| src/_native/misc.cpp | Added native functions (get_current_package, read_alias_package, etc.) |
| _msbuild_test.py, _msbuild.py | Updated build scripts to include the new native functions |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I think the only way to validate this is to ship it, since it's really hard to test without being able to install it. I've adjusted the release script to allow publishing without forcing everyone to auto-update, so we can have some time to test it directly without imposing it on everyone else. I'm going to do a bit more manual testing myself before merging though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a “first run” interactive configuration helper that prompts users to update key system settings (app execution aliases, long-path support, PATH entries, and Python installs) when they run the manager for the first time. It also introduces native APIs for querying package aliases and broadcasting environment changes, plus a new helper to enable long-path support.
- Add
first_runcommand class, CLI wiring, and calls inmanage/commands.py - Implement
first_runlogic insrc/manage/firstrun.pywith logging and user prompts - Expose native functions (
get_current_package,read_alias_package,broadcast_settings_change) andconfigure_long_pathin C++
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_logging.py | New tests for wrap_and_indent helper |
| tests/test_firstrun.py | Tests covering firstrun checks and commands |
| tests/conftest.py | Extend log-capture fixture with end_of_log |
| src/pymanager/main.cpp | Added configure_long_path and CLI hook for **configure-long-paths |
| src/manage/logging.py | Added wrap_and_indent and wrap support in LOGGER.print |
| src/manage/install_command.py | Only print shortcuts in non-automatic mode |
| src/manage/firstrun.py | Implement the first-run checks, prompts, and actions |
| src/manage/commands.py | Register **first_run and --configure options in CLI schema |
| src/_native/misc.cpp | New native exports for package/alias reading and settings broadcast |
| scripts/test-firstrun.py | Helper script to run firstrun.py without a rebuild |
| _msbuild.py / _msbuild_test.py | Include new native functions in build definitions |
Comments suppressed due to low confidence (1)
src/pymanager/main.cpp:85
- The new 'configure_long_path' function is not covered by any automated tests; consider adding a unit or integration test to verify registry key creation and error handling.
r = RegCreateKeyExW(HKEY_LOCAL_MACHINE, L"System\\CurrentControlSet\\Control\\FileSystem",
src/manage/commands.py
Outdated
| "check_long_paths": (config_bool, None, "env"), | ||
| "check_py_on_path": (config_bool, None, "env"), | ||
| "check_any_install": (config_bool, None, "env"), | ||
| "check_default_tag": (config_bool, None, "env"), |
Copilot
AI
May 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'check_default_tag' option is declared in the CLI schema but never used in the first_run handler; remove it or implement its behavior to keep the configuration schema in sync with the code.
| "check_default_tag": (config_bool, None, "env"), | |
| # Removed the unused "check_default_tag" option to keep the schema in sync with the code. |
Fixes #93