1

please help! I was looking for the answer all over the internet.

Here's my code:

@echo off
title var test
:question
set a1=This
set a2=Is
set a3=a
set a4=Var
set a5=Test
choice /c 12345 /m "press a number"
if errorlevel=5 set num=5&goto answer
if errorlevel=4 set num=4&goto answer
if errorlevel=3 set num=3&goto answer
if errorlevel=2 set num=2&goto answer
if errorlevel=1 set num=1&goto answer
:answer
echo now change the answer.
set /p a%num%=
FOR /F "tokens=1-6" %%1 IN ("%a1% %a2% %a3% %a4% %a5% a%num%") DO echo %%1 %%2 %%4 %%5.&echo You typed=%%6
pause
goto question

As you can see I made the user select a number between 1 and 5 to change the specific word. But when I try same kind of code to show what he typed doesn't work :(

Compo
  • 36,585
  • 5
  • 27
  • 39
Doug
  • 13
  • 6
  • 2
    For loops use %% not number. – Noodles Feb 28 '19 at 05:10
  • I know, but numbers from 1 to 9 works as well. – Doug Feb 28 '19 at 05:39
  • 1
    no, don't use numbers, these are assigned by default to input variables `%1`... anyway, your real problem is with the `if errorlevel` statements. try rather `if errorlevel 5 set..` by losing the = sign. Or even using `%errorlevel%` as `if "%errorlevel%"=="5" do...` you also have numerous other issues here and honestly do not even need thefor loop. – Gerhard Feb 28 '19 at 06:13
  • @GerhardBarnard, the `=` in this `if` statements is nothing but a standard token separator (just like the space), just to be precise... ;-) Anyway, `if ErrorLevel 1` actually means *if ErrorLevel is **greater than or equal to** 1* (type `if /?`), so these `if` statements needed to be put in reverse order. After the `if` statements, an error case should also be catched (like `goto question`, for instance, because `choice` returns an `ErrorLevel` of `0` if Ctrl+C is pressed, which would currently cause the `:answer` section to be executed)... – aschipfl Feb 28 '19 at 10:14

1 Answers1

2

Environment variables should never begin with a digit and using a digits for loop variables should be also avoided. Run in a command prompt window call /? and output is the help for this command explaining how batch file arguments can be referenced with %0, %1, %2, ... which explains why environment variables with digit as first character and loop variables with a digit are not good in general even on being a%num% in set of FOR does not reference the value of environment variable a1 or a2 or a3 or a4 or a5. It is just the name of the environment variable. The for loop is not necessary at all.

@echo off
title var test
:question
set "a1=This"
set "a2=Is"
set "a3=a"
set "a4=Var"
set "a5=Test"
%SystemRoot%\System32\choice.exe /C 12345E /N /M "Press a number in range 1-5 or E for exit: "
if errorlevel 6 goto :EOF
set "num=%ERRORLEVEL%"
set /P "a%num%=Now change the answer: "
echo %a1% %a2% %a3% %a4% %a5%.
call echo You typed: %%a%num%%%
pause
goto question

The command line call echo You typed: %%a%num%%% is parsed by Windows command processor before execution of the command line on number 3 entered to call echo You typed: %a3%. This command line is parsed a second time because of command call resulting in replacing %a3% by the value of environment variable a3 and so echo outputs the expected string.

It would be also possible to replace call echo You typed: %%a%num%%% by

setlocal EnableDelayedExpansion
echo You typed: !a%num%!
endlocal

The usage of delayed environment variable expansion results also in double parsing the command line before execution of command echo. For more details see How does the Windows Command Interpreter (CMD.EXE) parse scripts?

Please read also this answer for details about the commands SETLOCAL and ENDLOCAL.

The two lines below in batch code above are also not really good taking into account that the user can really enter anything.

echo %a1% %a2% %a3% %a4% %a5%.
call echo You typed: %%a%num%%%

For example if the user enters number 1 and on next prompt enters:

Your user name is:& setlocal EnableDelayedExpansion & echo !UserName!& endlocal & rem

Then the batch file does something completely different than designed for and outputs the user's account name.

Secure would be the batch code:

@echo off
title var test
setlocal EnableExtensions DisableDelayedExpansion
:question
set "a1=This"
set "a2=Is"
set "a3=a"
set "a4=Var"
set "a5=Test"
%SystemRoot%\System32\choice.exe /C 12345E /N /M "Press a number in range 1-5 or E for exit: "
if errorlevel 6 goto :EOF
set "num=%ERRORLEVEL%"
set /P "a%num%=Now change the answer: "
setlocal EnableDelayedExpansion
echo !a1! !a2! !a3! !a4! !a5!.
echo You typed: !a%num%!
endlocal
pause
goto question

Now the user input string cannot modify anymore the command lines executed by Windows command processor.

A solution with the useless FOR loop would be:

setlocal EnableDelayedExpansion
for /F tokens^=1-6^ eol^= %%A in ("!a1! !a2! !a3! !a4! !a5! !a%num%!") do echo %%A %%B %%C %%D %%E.&echo You typed: %%F
endlocal

eol= is necessary to output everything correct also if use enters number 1 and next a string starting with a semicolon. The FOR options string cannot be enclosed in double quotes in this case like "tokens=1-6 eol=" because of this would define " as end of line character and nothing is output if user enters number 1 and enters next a string starting with ". The equal sign and the space must be escaped with ^ to be interpreted as literal characters by cmd.exe on double parsing the entire for command line before execution of command for.

Note: The FOR loop solution does not work correct on user enters for first variable value the special command line string as posted above. So it is also not really secure.

Mofi
  • 46,139
  • 17
  • 80
  • 143