0

I'm making a Minecraft modding tool using a batch file. But on execution of the batch file the Windows command interpreter outputs the syntax error message:

) was unexpected

I can't figure out why.

This is my code:

@echo off
cd mods
setlocal enabledelayedexpansion
set "selected=1"
call:print 1
call:print 2
:menu
choice /c wse>nul
if "%errorlevel%"=="2" (
    if not !selected! GEQ !a! (
        set /a "selected+=1"
        cls
        call:print 1
        call:print 2
    )
)
if "%errorlevel%"=="1"  (
    if not !selected!==1 (
        set /a "selected-=1"
        cls
        call:print 1
        call:print 2
    )
)
if "%errorlevel%"=="3" (

)
goto menu

:print
if "%1"=="1"set a=0
echo.
if "%1"=="1" (
    echo Uninstalled:
) else (
    echo Installed:
)
echo.
for %%f in (*.jar) do (
    if "%1"=="1" (
        if NOT EXIST        
"C:/Users/Coornhert/AppData/Roaming/.minecraft/mods/%%~nf.jar" (
            set /a "a+=1"
        if "!a!"=="!selected!" (
            echo -%%~nf
        ) else (
            echo  %%~nf
        )
        set "b=!a!"
    )
) else (
    if EXIST "C:/Users/Coornhert/AppData/Roaming/.minecraft/mods/%%~nf.jar" (
        set /a "a+=1"
        if "!a!"=="!selected!" (
            echo -%%~nf
        ) else (
                echo  %%~nf
            )
            set "b=!a!"
        )
    )

)
goto :eof

And it works, but when I hit s, execution terminates with the error message.

Folder structure of folder containing the batch file:

  • mods
    • Foo.jar
    • Foo2.jar

Folder structure of target folder:

  • C:\Users\Coornhert\AppData\Roaming\.minecraft\mods
    • Foo.jar
Mofi
  • 38,783
  • 14
  • 62
  • 115
Pieter Mantel
  • 113
  • 1
  • 8
  • 1
    You haven't set `a`, so `!a!` (or `%a%`, which would be the same at that point) would be *nothing* hence the instruction is resolved as `if not 1 GEQ (` which is a syntax error. The classic solution is to "quote each term" in the comparison. – Magoo Aug 11 '17 at 14:02
  • I have quoted !a! but now it closes at goto :eof... – Pieter Mantel Aug 11 '17 at 14:41
  • I can see no reason why it should close. Since `:print` is a `call`ed subroutine, the `goto :eof` should return to the instruction following the `call`. There appears to be no point in setting `b` since it's not used and what `a` is supposed to do or its various values represent, I'm not sure (use `rem` to insert comments.) The `/` in the filepath should be backslash. The return `errorlevel` from `:print` should be `0` afaics - is the return value not `0` and causing another branch to be taken from the mainline? Commenting-out the `@echo off` would give you a step-by-step diagnostic trace. – Magoo Aug 11 '17 at 19:46

1 Answers1

0

I partly do not understand what this batch file should do, but here is the batch file rewritten with several improvements.

@echo off
setlocal EnableExtensions EnableDelayedExpansion
rem cd /D "%~dp0mods"
pushd "%~dp0mods"
set "a=0"
set "selected=1"
call :PrintIt 1
call :PrintIt 2

:Menu
choice /C wse /N
if errorlevel 3 popd & endlocal & goto :EOF
if errorlevel 2 goto AddOne

if %selected% == 1 goto Menu
set /A selected-=1
cls
call :PrintIt 1
call :PrintIt 2
goto Menu

:AddOne
if %selected% GEQ %a% goto Menu
set /A selected+=1
cls
call :PrintIt 1
call :PrintIt 2
goto Menu

:PrintIt
if %1 == 1 set "a=0"
echo/
if %1 == 1 (echo Uninstalled:) else echo Installed:
echo/

for %%I in (*.jar) do (
    if %1 == 1 (
        if not exist "%APPDATA%\.minecraft\mods\%%~nI.jar" (
            set /A a+=1
            if !a! == %selected% (echo -%%~nI) else echo  %%~nI
            set "b=!a!"
        )
    ) else (
        if exist "%APPDATA%\.minecraft\mods\%%~nI.jar" (
            set /A a+=1
            if !a! == %selected% (echo -%%~nI) else echo  %%~nI
            set "b=!a!"
        )
    )
)
goto :EOF

It does nothing useful as is, but batch code in question is also not useful at all.


The applied improvements are:

The command SETLOCAL is moved to top of file. The reason is:

  1. It pushes path of current directory on stack.
  2. It pushes state of command extensions on stack.
  3. It pushes state of delayed expansion on stack.
  4. It pushes the memory address of the current environment variables table on stack.
  5. It creates a copy of the current environment variables table in memory and makes this new environment variables table active.
  6. It sets command extensions and delayed expansion according to the specified parameters if the command is called with parameters at all.

The command ENDLOCAL is executed before leaving batch file. The reason is:

  1. It deletes the current environment table which means no environment variable defined in this batch file exists anymore after ENDLOCAL except it existed already before execution of command SETLOCAL.
  2. It pops memory address of previous environment table from stack and uses this address resulting in restoring initial environment variables.
  3. It pops state of delayed expansion from stack and disables/enables delayed expansion accordingly.
  4. It pops state of command extensions from stack and disables/enables command extensions accordingly.
  5. It pops previous current directory path from stack and sets current directory to this path to restore the current directory.

So the entire command process environment is restored on exit of this batch file to exactly the same environment as it was on starting the batch file.

This makes it possible to call this batch file from within another batch file or from within a command prompt window with no impact on calling batch file or command process.


The command CD could be extended to include drive and path of argument 0 which is the full path of the batch file ending with a backslash because the subdirectory mods is most likely always expected in directory of the batch file and it should not matter what is the current directory on running the batch file.

But cd /D "%~dp0mods" could fail if the batch file is located on a network share accessed using UNC path and therefore command PUSHD is used instead working with enabled command extensions also for UNC paths.


In all programming and scripting languages it is required that variables are defined and initialized with a value before being used the first time. For that reason the environment variables a and selected are defined at top of the batch file with default values. By the way: a is a very bad name for a variable. Why? Search for a in batch file. It is quite often found on not using special find features like whole word only, isn't it.


PRINT is a command as it can be seen on running in a command prompt window print /?. While it is possible to use command names as labels or as names for subroutines, it is not advisable to do so as it could be confusing for readers of the batch file.


The command CHOICE has the option /N to hide the list of choices in the prompt. It is better to use this option than redirecting the output of CHOICE to device NUL.


The very old but still valid Microsoft support article Testing for a Specific Error Level in Batch Files explains that if errorlevel X means that the condition is true if the exit code of previous command or application is greater or equal X. The command CHOICE with 3 choices exits always with 1, 2 or 3 as exit code. So it is best to use:

if errorlevel 3 rem Do something on third choice avoiding fall through to next line.
if errorlevel 2 rem Do something on second choice avoiding fall through to next line.
Do something on first choice.

The advantage of using this method is that it even works with CHOICE within a command block on which if %ERRORLEVEL% == X fails because of delayed expansion would be required and usage of if !ERRORLEVEL! == X.


The integer comparison if %selected% GEQ %a% would not work if the two arguments would be enclosed in double quotes as the double quotes are also interpreted as part of the arguments to compare. For that reason using if "%selected%" GEQ "%a%" would result in running a string comparison instead of an integer comparison. For more information about comparing values with IF look on answer on Exit a for loop in batch.

It is safe here to omit the double quotes also on the other IF conditions with == operator running string comparisons because the environment variables selected and a must be both defined before running this IF condition and therefore both variables are defined at top of the batch file.

The answer on Why is no string output with 'echo %var%' after using 'set var = text' on command line? explains why set "variable=value" should be always used to assign a value to an environment variable or delete an environment variable on omitting the value. And this answer also explains why on set /A variable=expression the double quotes can be usually omitted as whitespace characters are interpreted completely different within an arithmetic expression. The exception is usage of set /A with 1 or more commands on same command line on which double quotes around variable=expression would be also needed.


The batch file should be exited when the batch file user enters e or E to take third choice. This could be done with just goto :EOF, or with exit /B which is an alias for goto :EOF, or with just exit which always exits entire command process independent on calling hierarchy which is not recommended. Windows command interpreter would implicitly restore the initial stack before finishing batch file execution. But it is nevertheless good coding practice to pop from stack with code which was pushed on stack before with code. For that reason there is used popd & endlocal & goto :EOF. See answer on Single line with multiple commands using Windows batch file for more information about usage of multiple commands on one command line.


The list of predefined environment variables of used user account is output on running in a command prompt window the command set. One predefined Windows environment variable is APPDATA with path to application data of current user account. This environment variable should be used instead of a fixed path to application data directory of user account.

And the directory separator on Windows is the backslash character \ and not slash character / as on Unix and Mac.


The usage of f as loop variable is not recommended as this is also a loop variable modifier. %%~f can be interpreted by Windows command interpreter as value of loop variable f without surrounding double quotes or as incomplete loop variable reference because of missing loop variable after %%~f which could be also interpreted as full file name of ?. So it is better to use # or $ as loop variable or upper case letters to avoid such a confusion on interpreting the loop variable reference. Loop variables are case-sensitive.


I prefer for IF conditions with an ELSE branch the coding style

if condition (
    command
) else (
    command
)

But here in this batch file with command being just a short ECHO command the code is better readable on being more compact with using:

if condition (echo short message) else echo other short message

Delayed expansion for an environment variable referenced within a command block started with ( and ending with matching ) is only needed if the environment variable is also modified in same command block. Therefore environment variable a must be referenced in body of FOR with usage of delayed expansion while environment variable selected can be referenced as usual because of not modified within this command block at all.


It is better to use echo/ to output an empty line instead of echo.. For the reason read the DosTips forum topic: ECHO. FAILS to give text or blank line - Instead use ECHO/


For a basic understanding of the used commands, open a command prompt window, execute there the following commands, and read entirely all help pages displayed for each command very carefully.

  • call /?
  • choice /?
  • cls /?
  • echo /?
  • endlocal /?
  • for /?
  • goto /?
  • if /?
  • popd /?
  • pushd /?
  • rem /?
  • set /?
  • setlocal /?
Mofi
  • 38,783
  • 14
  • 62
  • 115