0

I wrote a batch script in which a query comes in an IF statement at the end, which sends the user back to the menu, terminates the script, or if the input is invalid, clears the console output and prompts the user again. The same happens at the end of the else statement.

If the user makes a choice now, he always has to do it twice. The first time the query is simply asked once again.

Does anyone have a solution for this?

Some parts of the menu:

:restart
cls
set /p input=enter your prefered number and press "enter":
if "%input%"=="1" ( goto :installsd 
) else if "%input%"=="4" ( goto :restoreup
) else if "%input%"=="6" ( goto :end 
) else ( goto :restart )

This works correctly:

:installsd
...some code...
    :A
    cls
    echo [1] back to menu
    echo [2] exit
    set /p input=enter your prefered number and press "enter":
    if "%input%"=="1" ( goto :restart 
    ) else if "%input%"=="2" ( goto :exit 
    ) else ( goto :A )

But here I need double input to come back to menu or exit.
After the first input it runs goto :B/goto :C and THEN I go back to menu.

:restoreup
if exist "%FilePath%backup\patch" (
...some code...
    :B
    cls
    echo [1] back to menu
    echo [2] exit
    set /p input=enter your prefered number and press "enter":
    if "%input%"=="1" ( goto :restart 
    ) else if "%input%"=="2" ( goto :exit 
    ) else ( goto :B )  
) else (
...some code...
    :C
    cls
    echo [1] back to menu
    echo [2] exit
    set /p input=enter your prefered number and press "enter":
    if "%input%"=="1" ( goto :restart 
    ) else if "%input%"=="2" ( goto :exit 
    ) else ( goto :C )
    )
Mofi
  • 46,139
  • 17
  • 80
  • 143

1 Answers1

3

I've never been a big fan of the:

if some_condition:
    go_somewhere_else
else:
    do_something

way of doing things. The else in those cases is totally unnecessary and clogs up what could be more readable code:

if some_condition:
    go_somewhere_else
do_something

That's especially true in a language like cmd where it wasn't so much designed as thrown together from scraps then fixed up over decades with band-aid solutions (such as the need for delayed expansion in complex statements like you have in your code).

Your specific problem is due to the fact that statements are read and evaluated in full before any part of them is executed. So consider your code segment:

if exist "%FilePath%backup\patch" (
    cls
    echo [1] back to menu
    echo [2] exit
    set /p input=enter your prefered number and press "enter":
    if "%input%"=="1"
    :
)

A statement here is everything from the first if to the ) that terminateses that if statement. That includes the %input% evaluation within it.

That means %input% is evaluated before executing the statement and therefore before the set /p that sets it to what you want it to be. Hence it will have the previous value that it was set to (or be empty if nothing had set it).

You can solve this problem by using delayed expansion as seen in the following example script:

@echo off
setlocal enableextensions enabledelayedexpansion

set var=previous_value
if 1==1 (
    set var=new_value
    echo percent %var%
    echo delayed !var!
)

endlocal

The output of that is:

percent previous_value
delayed new_value

So you can see the the % version is the old value at the time the if statement was evaluated, while the ! version was evaluated at the time that line was executed (not the statement).


I usually try to avoid cmd for complex stuff, especially since modern Windows has Powershell, a much more usable language in my opinion (though a little on the verbose side).

If you're going to write code in cmd, it's best to keep it as simple as possible, such as re-doing your if statements to a more simplified form. For example, your first code block would be:

:restart
    cls
    set /p input=Enter your preferred number:
    if "%input%"=="1" goto :installsd 
    if "%input%"=="4" goto :restoreup
    if "%input%"=="6" goto :end 
    goto :restart

A separate area where you need another loop would then be something like:

:restoreup
    if not exist "%FilePath%backup\patch" goto :ru_nofile
    rem ...some code...
:ru_menu1
    cls
    echo [1] back to main menu
    echo [2] exit
    set /p input=Enter your preferred number:
    if "%input%"=="1" goto :restart 
    if "%input%"=="2" goto :exit 
    goto :ru_menu1

:ru_nofile
    rem ...some code...
:rm_menu2
    cls
    echo [1] back to menu
    echo [2] exit
    set /p input=Enter your preferred number:
    if "%input%"=="1" goto :restart 
    if "%input%"=="2" goto :exit 
    goto :ru_menu2

And, yes, it looks more like assembler (or very old BASIC) code but it at least ensures that there is a one-to-one correspondence between lines and statements, meaning you won't get bitten by non-delayed expansion.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953