bpo-30362 : Add list options to launcher. #1578
Conversation
|
@GadgetSteve, thanks for your PR! By analyzing the history of the files in this pull request, we identified @briancurtin, @zooba and @vsajip to be potential reviewers. |
GadgetSteve
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1578 +/- ##
=========================================
Coverage ? 82.69%
=========================================
Files ? 1432
Lines ? 353213
Branches ? 0
=========================================
Hits ? 292073
Misses ? 61140
Partials ? 0Continue to review full report at Codecov.
|
…uncher-list and --launcher-list-paths
|
I agree this is a useful feature, but I'm not sure I like the patch stylistically. Haven't yet had time to look at it in more detail. |
| else | ||
| { | ||
| for (i = 0; i < num_installed_pythons; i++, ip++) { | ||
| fwprintf(stdout, fmt, ip->version, ip->bits, ip->executable); |
There was a problem hiding this comment.
Does it really need to print the executable path? It's trivial to get that information manually.
PC/launcher.c
Outdated
| if (ip == NULL) | ||
| fwprintf(stderr, L"\n\nCan't find a Default Python.\n\n"); | ||
| else | ||
| fwprintf(stderr, \ |
There was a problem hiding this comment.
Instead of printing the default version to stderr, it could indicate the default version in the list with an asterisk. For example, *3.6-64.
PC/launcher.c
Outdated
| if (argc == 2) { | ||
| slen = wcslen(L"-0"); | ||
| if(!wcsncmp(p, L"-0", slen)) /* Starts with -0 */ | ||
| show_python_list(argv); /* Check for -0 FIRST */ |
There was a problem hiding this comment.
Can we return the exit code from show_python_list rather than using exit() in it? It's probably safe enough (because I'm sure enough people have done it in the past that the MS CRT protects itself against people doing it), but better to return from the entry point than to force an exit.
There was a problem hiding this comment.
Restructured to allow exit via return from main rather than direct exit call.
PC/launcher.c
Outdated
| } | ||
| } | ||
|
|
||
| ip = locate_python(L"", FALSE); | ||
| /*ip = locate_python(L"", FALSE);*/ |
There was a problem hiding this comment.
Remove this if it's not needed now.
|
Please also see https://devguide.python.org/committing/#what-s-new-and-news-entries and add a NEWS and what's new item for this. |
Removed commented out line of code in PC/launcher.c. Added the results of using blurb to add details of bpo-30362 & bpo-30291. Updated Doc/whatsnew/3.7.rst to add a Windows only section covering both tickets.
|
I am unable to resolve conflict in Doc/whatsnew/3.7.rst |
|
Conflict resolved. |
|
Thanks. I shortened the two NEWS items so they are in line with the rest of our items. Once the checks are complete then I'll merge. Thanks for this! |
These changes add to the windows launcher the options to list the available pythons, (-l or --list), or list with the paths, (-L or --long-list), and then terminate.
In the process the output of the help messages has been moved to a distinct function, (show_help_text).
Note that informational messages are sent to stderr and only the possible -x.y-bb flags or the flags and paths are output to stdout, this is intended to allow pipe of the flags for onward processing or scripting.