Batch file loop issues

Question

I am having loop issues in my batch file. When using the set command, I give the user two choices. The first option works fine however the second option is ignored unless a loop function is done to re-ask the question. Then it accepts the input for the second option.

:WithAccess
cls
If not exist "C:\Program Files (x86)\Microsoft Office\Office15\Winword.exe" (
    goto UninstallViewers
    :WithAccess2
    echo Installing MS Office 2013 x32 With Access...
    start /wait "" "MS Office 2013 x32 Installers\MSOfficeWithAccess"
    echo Installation Complete.
) Else (
    echo Microsoft Office 2013 might already be installed.
    set /p op4=Do you want to run installer anyway? [Y/N]:
    If "%op4%"=="y" goto WithAccess2
    If "%op4%"=="n" goto end
    echo That's not a valid option.
    goto WithAccess
)

So, in the "Else" part of this if statement, when the user chooses option 2 or "n" it will ignore this and move onto whatever code is after it. In this case it loops back to the beginning of this code segment. However after the loop, it now accept the user input and correctly jumps to the "end" label.

After playing around I found that removing the labels surrounding this segment fixes it, but that makes it so if the user makes an error, it wont loop back and re-ask the question. Originally my "Else" statement looked like this:

Else (
    echo Microsoft Office 2013 might already be installed.
    :Loop2
    set /p op4=Do you want to run installer anyway? [Y/N]:
    If "%op4%"=="y" goto WithAccess2
    If "%op4%"=="n" goto end
    echo That's not a valid option.
    goto Loop2
)

So that it would ideally loop the same question but obviously the same issue occurred. I've tried searching around for the answer but its hard to search for such a specific coding issue. Any help would be appreciated.

Answer 1

It's bad practice to put labels within parenthetical code blocks. If you goto label and that label is within a code block, the thread of execution lands on the label thinking it is no longer within a code block, and problems arise. You should re-work the logic of your script's flow. Move your labels outside any parentheses, and consider using call rather than goto when you want to call a function then return to continue parsing the next line. You might even find it useful to return values from your functions from time to time. Also, use if /i for case insensitive tests.

:WithAccess
cls
If not exist "C:\Program Files (x86)\Microsoft Office\Office15\Winword.exe" (
    call :UninstallViewers
) Else (
    echo Microsoft Office 2013 might already be installed.
    set /p "op4=Do you want to run installer anyway? [Y/N]: "
    setlocal enabledelayedexpansion
    If /I "!op4!"=="y" goto WithAccess2
    If /I "!op4!"=="n" goto :EOF
    endlocal
    echo That's not a valid option.
    goto WithAccess
)

:WithAccess2
echo Installing MS Office 2013 x32 With Access...
start /wait "" "MS Office 2013 x32 Installers\MSOfficeWithAccess"
echo Installation Complete.

:: end main runtime
goto :EOF

:UninstallViewers
:: (or whatever code you have to do the uninstalling)
wmic product where "name like '%%viewer%%' and vendor like '%%microsoft%%'" call uninstall
goto :EOF