2

So I have this subroutine that I want to call from another location in a batch file. The functions work as desired, but for some reason I cant pin down, the prompt wants to have the user enter something TWICE, before it will accept anything.

Say, if I enter "0", to go back to a previous menu, it takes me right back to the prompt, and I have to enter "0" again before it will actually go back to the previous menu (elsewhere in my main script).

I can, say, enter "w" (or any other value), then the second time, enter the one I actually WANT to use, and it will finally do it.

This is driving me nuts.

:subfullbackup
cls

if exist "%current%\Backup\Full_Backup" (
  Echo  Backup folder already exists
  Echo.
  Echo  [o]  Overwrite local device files with existing local files
  Echo  [w]  Wipe current local backup and start fresh
  Echo.
  set /p choice=Select: 


  if %choice% == o (
    Echo.
    Echo  Depending on how much data you have,
    Echo  this could take a couple hours.
    Echo.
    Echo  Backing up...
    adb pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  -= BACKUP COMPLETE =-
    Pause
    Goto :backup
  )


  if %choice% == w (
    Echo.
    Echo  Removing all current local backup files in 'Full_Backup'
    rmdir /S /Q "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  Depending on how much data you have,
    Echo  this could take a couple hours.
    Echo.
    Echo  Backing up...
    adb pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  -= BACKUP COMPLETE =-
    Pause
    Goto :backup
  )


  if not %choice% == o goto subfullbackup
  if not %choice% == w goto subfullbackup
) else (
    Echo.
    Echo  Depending on how much data you have,
    Echo  this could take a couple hours.
    Echo.
    Echo  Backing up...
    adb pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  -= BACKUP COMPLETE =-
    Pause
    Goto :backup
)
Goto :eof
Noah
  • 93
  • 2
  • 9
  • 2
    `%Variables%` inside `()` block are expanded before the entire block is executed, so on the first run `if not %choice% == o goto subfullbackup` goes to the beginning and displays the prompt again. See [this](http://stackoverflow.com/a/33817417/3959875) – wOxxOm Nov 29 '15 at 00:06
  • 1
    [maybe a bit better to understand](http://stackoverflow.com/a/30284028/2152082) – Stephan Nov 29 '15 at 08:22

1 Answers1

1

Your batch code with using delayed expansion, enabled at top of the batch script with command setlocal which additionally creates a copy of all environment variables and remembering also current directory for restoring the variables list, current directory and current states of command extensions and delayed expansion on endlocal or leaving batch processing:

@echo off
setlocal EnableDelayedExpansion
set "current=%CD%"

:FullBackup
cls

if exist "%current%\Backup\Full_Backup" (
  Echo  Backup folder already exists
  Echo.
  Echo  [o]  Overwrite local device files with existing local files
  Echo  [w]  Wipe current local backup and start fresh
  Echo.
  set "UserChoice="
  set /p "UserChoice=Select: "


  if /I "!UserChoice!" == "o" (
    Echo.
    Echo  Depending on how much data you have,
    Echo  this could take a couple hours.
    Echo.
    Echo  Backing up...
    adb.exe pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  -= BACKUP COMPLETE =-
    Pause
    Goto DoBackup
  )


  if /I "!UserChoice!" == "w" (
    Echo.
    Echo  Removing all current local backup files in 'Full_Backup'
    rmdir /S /Q "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  Depending on how much data you have,
    Echo  this could take a couple hours.
    Echo.
    Echo  Backing up...
    adb.exe pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  -= BACKUP COMPLETE =-
    Pause
    Goto DoBackup
  )

  goto FullBackup

) else (
    Echo.
    Echo  Depending on how much data you have,
    Echo  this could take a couple hours.
    Echo.
    Echo  Backing up...
    adb.exe pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
    Echo.
    Echo  -= BACKUP COMPLETE =-
    Pause
    Goto DoBackup
)
Goto :EOF

:DoBackup

But your batch code could be also written without delayed expansion and much more compact avoiding duplicate code lines:

@echo off
set "current=%CD%"

:FullBackup
cls

if exist "%current%\Backup\Full_Backup" goto PromptBackup

:OverwriteBackup
Echo.
Echo  Depending on how much data you have,
Echo  this could take a couple hours.
Echo.
Echo  Backing up...
adb.exe pull /sdcard/ "%current%\Backup\Full_Backup" >nul 2>&1
Echo.
Echo  -= BACKUP COMPLETE =-
Pause
Goto DoBackup

:PromptBackup
Echo  Backup folder already exists
Echo.
Echo  [o]  Overwrite local device files with existing local files
Echo  [w]  Wipe current local backup and start fresh
Echo.
set "UserChoice="
set /p "UserChoice=Select: "

if /I "%UserChoice%" == "o" goto OverwriteBackup

if /I not "%UserChoice%" == "w" goto FullBackup

Echo.
Echo  Removing all current local backup files in 'Full_Backup'
rmdir /S /Q "%current%\Backup\Full_Backup" >nul 2>&1
goto OverwriteBackup

:DoBackup

Some notes about small changes in code:

  1. choice (SS64 article) is a standard Windows command. Therefore it is advisable to avoid choice (Microsoft article) as name for an environment variable or label. UserChoice (CamelCase spelling for easier reading) is used instead of choice.

  2. backup (SS64 article) is not a standard Windows command, but a standard SQL command. Therefore it is also advisable to avoid backup as name for an environment variable or label. DoBackup is used instead in batch code above.

  3. It is advisable to define a default for an environment variable before prompting a user. The user can hit just RETURN or ENTER in which case the environment variable keeps its value.

    The environment variable is cleared with set "UserPoint=" before prompting the user and therefore the variable does not exist when user enters nothing.

    Possible would be also set "UserPoint=o" or set "UserPoint=w" to define a valid default value.

  4. Comparing strings with user input should be done always with using double quotes to avoid an exit of batch processing caused by a syntax error when user inputs nothing.

    if %choice% == w ( becomes if == w ( when the user enters nothing which is a syntax error and results in breaking batch processing by command processor.

    if /I "%UserChoice%" == "w" ( becomes if /I "" == "w" when the user enters nothing in code above which is still valid batch code and can be therefore processed.

    Note: User could now break batch processing by entering "w".
    But it can be expected here that the user does not input 1 or more double quotes on being asked for o or w.

  5. On comparing strings entered by user with predefined strings it is advisable to do case-insensitive string comparisons if letters are included in the compared strings.

    The option /I changes a string comparison from case-sensitive to case-insensitive.

    So now the user can enter also O or W and this is interpreted like o or w.

Mofi
  • 46,139
  • 17
  • 80
  • 143