Implement enhanced Windows make.bat (#3741) #3841
Conversation
(COMMIT 1) DEV: Draft Win make.bat multi-argument support Windows 'make.bat' created by sphinx-quickstart.py now is able to handle multiple build targets and also sphinx-build command line options (http://www.sphinx-doc.org/en/stable/man/sphinx-build.html#options). Modified make.bat properly handles options that take an value, though it REQUIRES there to be a space between the option flag and the value passed. (COMMIT 2) DEV/ADMIN ADMIN: Add 'feature added' entry for the enhanced make.bat DEV: make.bat.new_t: Some new stuff added; not fully done yet * Change so that the *last* option character passed determines whether an extra argument is to be absorbed from the invocation. * Add KEYVALUE env var, in anticipation of implementing the key/value detection/handling code * Remove some redundant zeroes in env var substringing * Fix laggard %HASARG% --> %TAKESARG% * Add printing of the make target currently being processed * Add exit-on-error behavior on a per-target basis make.bat_t: Parallel changes as to make.bat.new_t test_winmake.bat: Batch-script "test suite" for the new make.bat * Make '-h' show help * Make '-k' not delete the temporary docset when testing done * Silent test running fouled by CMD misbehavior; see "Buggy behaviour when using CALL" ~2/3 of the way down on https://ss64.com/nt/call.html * Creates new Sphinx docset with sphinx-quickstart.py * Runs various EXPECT-SUCCESS and EXPECT-FAIL tests * Print test summary report at end of execution. (COMMIT 3) DEV: Test suite test_winmake.bat: * Add a VERBOSE option w/message in help display * Refactor testing to subroutine with ~callback to each test run. Communication between test setup and test runner is by specified variables * Change format of pass/fail in test report for better visibility (COMMIT 4) DEV: Fix bug, restructure output All in test_winmake.bat Bug: Add missing '%' in VERBOSE mode 'make' call Output changes: * Define output "headers" for unified printing of test output * Add notification for if sandbox docset left in place (COMMIT 5) REVERT: Restore unmodified make.bat_t (COMMIT 6) ADMIN: .gitignore and -h display * Add tests/winmake/testdir to .gitignore, so that any retained test folder is ignored * Change test_winmake.bat label/goto structure so that extraneous/ irrelevant testing-related output is not printed when called with '-h' (COMMIT 7) DEV: Mainly %SILENT%, %ANYFAILED% make.bat.new_t: * Change from %0 to explicit make when making the recursive calls. Was some path weirdness causing these calls to fail. * Fix %ERRORLEVEL% check by changing to numeric test test_winmake.bat: * Add %SILENT% and handling of '-s' flag; roll out output suppression if '-s' supplied * Add %ANYFAILED%, as 'sticky' flag for failure at any point in the series of tests; add exit based on %ANYFAILED% value * Exit with error if both '-v' and '-s' are supplied * Add tests: * make latex * make clean html * make html -a * make html -E (COMMIT 8) DEV: Mod for key-value args; add tests make.bat.new_t: * Change -D and -A to be args that take key-value pairs by storing %TAKESARG% with a value of 2 * Implement argument consumption for "%TAKESARG%"=="2" arguments that properly passes the args to sphinx-build with the proper key=value structure test_winmake.bat: * Add tests: * make html -Ec . * make clean html -Ec . * make clean html epub * make clean html epub -E * make clean html -D rst_epilog="**THIS**" * Update formatting of test summary to handle wider column needed for the -D rst_epilog test case (COMMIT 9) TESTS: More lumped multi-option test cases test_winmake.bat: * Add OK test 'make html -qWE', confirming works ok for lumped multi-option arguments without a trailing argument value * Add ERROR test 'make html -qW.E', which fails since '-.' is not a valid argument to sphinx-build (COMMIT 10) ADMIN/DEV/DOC AUTHORS (ADMIN): Add name doc/invocation.rst (DOC): * Minor edits to paragraph introducing environment variables used by ``sphinx-build`` * Added exposition on Linux `make` and Windows `make.bat', including description of the new capabilities added to make.bat templates/quickstart/make.bat.new_t (DEV): * Change the exit approach for the (disallowed) `-b` option, to use the more proper `exit /b 1` syntax after a new `:end_error` label * Add comment that the arg parsing needs to be updated if new options are added that take values tests/winmake/test_winmake.bat (ADMIN): * Change header in test results to full word "Arguments" * Expand info in 'help' display to note that tests don't check for whether the build operation completed as intended per the arguments passed, but instead just test for a success or failure exit ERRORLEVEL. * Clarify help as re the `-s` option * Add paragraph to help recommending running the tests from within a freshly created virtualenv * Indicate that this batch file exits with ERRORLEVEL==0 if all tests pass; ERRORLEVEL==1 otherwise
|
@shimizukawa could you check this please? |
| if "%WORK:~-1%"=="D" set TAKESARG=2 | ||
| if "%WORK:~-1%"=="t" set TAKESARG=1 | ||
| if "%WORK:~-1%"=="A" set TAKESARG=2 | ||
| if "%WORK:~-1%"=="w" set TAKESARG=1 |
There was a problem hiding this comment.
Are these options for sphinx-build command?
If so, make.bat knows too much for its responsibility.
There was a problem hiding this comment.
Since following changes of sphinx-build command is too heavy, I suggest that make.bat should accept arguments of the following pattern.
make.bat <target>... <sphinxopts>
make.bat do not check and modify<sphinxopts> part at all, then pass it to each sphinx-build commands for (maybe more than one) targets.
If <sphinxopts> contains wrong options or option arguments, sphinx-build will check and inform us.
There was a problem hiding this comment.
@cocoatomo Yes, this is for handling of the options to be passed through to sphinx-build and I agree that it's fragile. I have no problem revising to implement the command structure you propose; I debated going with basically this same thing at first.
Would the logic be sound if I split the arguments into build targets vs sphinxopts based upon the first occurrence of a hyphen in the command invocation? It looks like all of the builder names are alpha characters only -- can I rely on this holding in the future?
Per [discussion](sphinx-doc#3841 (comment)) on PR sphinx-doc#3841, re-implemented the argument parsing to eliminate fragility of explicit handling of `sphinx-build` arguments. Syntax now must follow: make target1 target2 ... <sphinxopts>
* TEST/ADMIN: Tweak help msg, new tests ADMIN: Improve language about how this doesn't test that docs are properly generated TESTS: * New expect-success test for call with extra spaces between build targets * New expect-failure tests for call with `sphinx-build` option placed before a build target * DEV: Complete rework of argument parsing Per [discussion](sphinx-doc#3841 (comment)) on PR sphinx-doc#3841, re-implemented the argument parsing to eliminate fragility of explicit handling of `sphinx-build` arguments. Syntax now must follow: make target1 target2 ... <sphinxopts> * DOC: Update invocation.rst Add content indicating the structural restriction of `make.bat` invocations, where any options for pass-through to `sphinx-build` must be passed after all build targets.
|
@cocoatomo Argument parsing has been revised - what do you think? |
make.bat.new_t: Alter the detection of the hyphen of the first option passed by looking for a space followed by a hyphen. This way, if a build target were to be created in the future that contains an internal hyphen, it wouldn't break argument parsing. Proper handling of argument-only calls to make.bat then require an initial check of the arguments to see if they start with a hyphen, since this case is no longer checked within the argument processing loop. test_winmake.bat: Add test for plain call to make.bat, with no arguments whatsoever.
|
@shimizukawa Any comments? From my side, this is too complicated for me. That might be related with that I'm not Windows developer. Anyway, I think your approval is needed to merge this. |
|
Note: I think this is an improvement. So I change this milestone to next major release. |
|
I think this change introduces unnecessary complexity, then future maintenance will definitely be difficult. |
|
@shimizukawa Hm - is there any way to bring in a proper As well, I'm unclear as to what about it introduces much complexity into future maintenance. Now that I've revised it per @cocoatomo's suggestions, it seems to me it should be robust against just about everything except a major re-work of the That said, the functionality isn't hugely important -- it's mainly a convenience feature. For my own purposes, it's sufficient just to add " |
|
@shimizukawa any comments? Note: I'll reject this if anyone says ok to this because it means we can't maintain this. |
|
I'm fine with this being closed w/o merge, @tk0miya. I understand the concerns about it representing significant additional complexity. Really, I got what I wanted out of it -- I know how to custom-mod my own Sphinx installs to streamline my workflow on Windows, and dramatically increased my Windows shell scripting skills to boot! Thanks for your efforts in evaluating this! (@cocoatomo and @shimizukawa too.) |
|
Thank you for your work! |
Feature or Bugfix
Feature
Purpose
make.batis only able to process a single build target on each invocation. This PR incorporates modifications to the Jinja template for the newmake.bat(the one that callssphinx-buildwith the-Moption) that enable (1) passing multiple build targets on a single invocation and (2) passingsphinx-buildoptions tomake.bat, which are then appended to thesphinx-buildcall to each build target.make.batworks for this author on Windows 7 Professional, 64-bit. It is anticipated to work on Win8 and Win10, and the functionality should be independent of the active Python version. This PR should have no effect whatsoever on the internal functionality of the Sphinx project itself - it deals entirely with the surroundingmake.batinvocation tool forsphinx-build.Detail
Relates
stablesince it does not represent a breaking change.