AnOldSoul
AnOldSoul

Reputation: 4207

Iterating through the files and extracting only if its a zip folder using a batch file

I need to iterate through all the files in the current folder and extract them only if its a zip file. So I wrote the below batch script. But I am getting an error saying

"=="."zip" was unexpected at this time

Below is the batch script I wrote.

@echo off
cd /d %~dp0
for /r %%i in (*) do if %%i "%Extension%"==".zip" (
setlocal
cd /d %~dp0
Call :UnZipFile "G:\NewUpdates\ExtractedStuff" %%i
exit /b

:UnZipFile <ExtractTo> <newzipfile>
set vbs="%temp%\_.vbs"
if exist %vbs% del /f /q %vbs%
>%vbs%  echo Set fso = CreateObject("Scripting.FileSystemObject")
>>%vbs% echo If NOT fso.FolderExists(%1) Then
>>%vbs% echo fso.CreateFolder(%1)
>>%vbs% echo End If
>>%vbs% echo set objShell = CreateObject("Shell.Application")
>>%vbs% echo set FilesInZip=objShell.NameSpace(%2).items
>>%vbs% echo objShell.NameSpace(%1).CopyHere(FilesInZip)
>>%vbs% echo Set fso = Nothing
>>%vbs% echo Set objShell = Nothing
cscript //nologo %vbs%
if exist %vbs% del /f /q %vbs%
)
pause

what am I doing wrong here? Please advice.

Upvotes: 0

Views: 2344

Answers (2)

aschipfl
aschipfl

Reputation: 34909

You are reading an undefined variable %Extension%, and you have two expressions on the left side of the == in the if statement, which is a syntax error.

To get the file name extension of an item iterated by a for loop, use the ~x modifier (type for /? on command prompt to find out more about that), like %%~xi in your code:

if /i "%%~xi"==".zip" ( ... )

The /i switch makes the comparison case-insensitive, which is recommended here as Windows treats file names in the same manner.


However, in your situation you do not need the if statement at all for checking the file name extension, because you can let for do that job:

for /r %%i in (*.zip) do ( ... )

This loop iterates through *.zip files only.


There is another issue in your code: the sub-routine :UnZipFile is part of the loop body, and so there is and exit /b inside of the loop.

The sub-routine must be outside of the loop. When call is executed, the current contexts of parenthesised blocks of code are not known in the sub-routine, so it will not be considered ended as soon as the loop-ending ) appears, rather a syntax error arises. Hence you need to move the ) from the very bottom to a new line before exit /b (if you place it after, the exit command will break the loop).

Since you have setlocal inside of your loop, you need to put also endlocal (immediately before the )) in order not to exceed the setlocal nesting limit. However, if you use full paths inside of the loop, you do not need setlocal/endlocal. Instead I would move setlocal/endlocal into the sub-routine to localise the environment variables used therein.


I am not sure if you intend to change to the directory where the script is stored by cd /d %~dp0, but I think you want the full path to the currently iterated *.zip file in your loop, right? If so, remove the cd command and use %%~fi in your call command line.


Finally, quotation is not optimal throughout the entire code. For example, it is always a good idea to quote file paths and names as they may contain white-spaces or some other characters that are considered as delimiters by the command interpreter or have some other special meaning to it.

Also in the sub-routine you should quote the paths provided as arguments, and access them as "%~1", for instance, to get them properly quoted (type call /? to see that the ~ removes surrounding quotes potentionally delivered by the command line).

And the best set syntax is: set "vbs=%temp%\_.vbs"; so the quotes do not become part of the value and you have always one place to put quotation marks -- namely when reading (expanding) the variable, like "%vbs%".


Finally, let me recommend you to use indention so that the code becomes much more legible and maintainable.


Here is the fixed code:

@echo off
cd /d %~dp0
for /r %%i in ("*.zip") do (
    Call :UnZipFile "G:\NewUpdates\ExtractedStuff" "%%~fi"
)
exit /b

:UnZipFile <ExtractTo> <newzipfile>
    setlocal
    set vbs="%temp%\_.vbs"
    if exist "%vbs%" del /f /q "%vbs%"
     >"%vbs%" echo Set fso = CreateObject("Scripting.FileSystemObject")
    >>"%vbs%" echo If NOT fso.FolderExists("%~1") Then
    >>"%vbs%" echo fso.CreateFolder("%~1")
    >>"%vbs%" echo End If
    >>"%vbs%" echo set objShell = CreateObject("Shell.Application")
    >>"%vbs%" echo set FilesInZip=objShell.NameSpace("%~2").items
    >>"%vbs%" echo objShell.NameSpace("%~1").CopyHere(FilesInZip)
    >>"%vbs%" echo Set fso = Nothing
    >>"%vbs%" echo Set objShell = Nothing
    cscript //nologo "%vbs%"
    if exist "%vbs%" del /f /q "%vbs%"
    endlocal
    pause

Upvotes: 2

prudviraj
prudviraj

Reputation: 3744

My advice is to simplify the search for zip files by filtering them in the for command , rather than check for file extensions

Use below command :

for /f "tokens=*" %%a in ('dir /a:-d /b ^| findstr /r ".zip$"') do echo %%a

The above command will list all zip files in your directory , now you can play with them the way you want

Hope this helps.

Upvotes: 0

Related Questions