#1 17 Jul 2019 08:35

Rekrul
Member
Registered: 17 Apr 2016
Posts: 32

I give up!!! Why doesn't this work???

Have I ever mentioned how much I absolutely F***ing HATE with an intense burning passion, the absolutely idiotic variable handling in batch scripts? I want to invent a time machine, go back in time, find the creator of this broken mess and strangle the life out of them!

I'm trying to update a counter within a loop. Because the loop goes through a directory of filenames and some of those names may contain "!", I can not enable delayed expansion for the entire loop. I have to safely store the filename into a variable before enabling it or any "!" in the name will be stripped out and any operations on that file will fail. This means that I also have to do ENDLOCAL before the loop repeats. OK, I have no problem with that. That all works fine.

The problem I have is that using ENDLOCAL completely breaks arithmetic operations!!!

Here is the code stripped down to just what I need to test this. The full script also performs operations on the filenames themselves, which is why I need to enable delayed expansion after the filename has been stored in a variable to ensure that it is preserved exactly the way it is on the disk. I've only defined the !extension! variable here because this is just an example that I'm trying to make work.

echo off

set /A c=0

for %%F in ("*.*") do (

set extension=%%~xF

setlocal enabledelayedexpansion

if "!extension!"==".jpg" set /A c=c+1

echo !c!

endlocal

)

It should count the number of jpeg files in the directory. Does it work? Of course not!!!

If I change it to;

echo off

set /A c=0

setlocal enabledelayedexpansion

for %%F in ("*.*") do (

set extension=%%~xF

if "!extension!"==".jpg" set /A c=c+1

echo !c!

)

It works as expected. However, I can't do this in the full script because the full script also performs operations on the name portion of the filename and it will fail if the name contains any "!", which will be stripped out.

So...

How do I create a FOR loop that can handle filenames with "!" in them, and make arithmetic variables work?

Offline

#2 17 Jul 2019 13:04

RG
Member
From: Minnesota
Registered: 18 Feb 2010
Posts: 348

Re: I give up!!! Why doesn't this work???

@Rekrul,
For the code you have shown you don't really need the intermediate variable 'extension', and therefore don't need delayed expansion at all.

echo off
set /A c=0
for %%F in ("*.*") do (
   if "%%~xF"==".jpg" set /A c+=1
   )
echo(c=%c%
pause

Windows Shell Scripting and InstallShield

Offline

#3 17 Jul 2019 18:06

Rekrul
Member
Registered: 17 Apr 2016
Posts: 32

Re: I give up!!! Why doesn't this work???

RG wrote:

@Rekrul,
For the code you have shown you don't really need the intermediate variable 'extension', and therefore don't need delayed expansion at all.

Here is the full script that I wanted to add a counter to;

echo off
cls
setlocal disabledelayedexpansion
echo.
echo Fixing incorrect Tumblr sizes...
echo.

for /R %%F in ("*tumblr*.*") do (
   for /F %%w in ('identify -quiet -format "%%w" "%%F[0]"') do (set width=%%w)

set name1=%%F
set name2=%%~nF
set extension=%%~xF

setlocal enabledelayedexpansion

set size=!name2:~-4!
set size=!size:_=!

if !width! gtr 640 set width=1280
if !width! leq 250 set width=250
if !width! gtr 540 (if !width! leq 640 (set width=640))
if !width! gtr 500 (if !width! leq 540 (set width=540))
if !width! gtr 400 (if !width! leq 500 (set width=500))
if !width! gtr 250 (if !width! leq 400 (set width=400))

if not "!size!"=="!width!" (
 for %%a in (!size!) do (for %%b in (!width!) do (set name2=!name2:%%a=%%b!))

ren "!name1!" "!name2!!extension!"
)
endlocal
)

setlocal disabledelayedexpansion
echo Fixing incorrect extensions...
echo.

for /R %%F in ("*.jpg", "*.jpeg", "*.png", "*.gif", "*.webp", "*.bmp") do (
   for /F %%m in ('identify -quiet -format "%%m" "%%F[0]"') do (set type=%%m)

set name1=%%F
set name2=%%~nF
set extension=%%~xF

setlocal enabledelayedexpansion

if "!type!"=="JPEG" set type=jpg
if "!type!"=="PNG" set type=png
if "!type!"=="GIF" set type=gif
if "!type!"=="WEBP" set type=webp
if "!type!"=="BMP3" set type=bmp

if /I not "!type!"=="!extension!" (
 set extension=!type!

ren "!name1!" "!name2!.!extension!"
)
endlocal
)

setlocal disabledelayedexpansion
echo Checking for 102+ character names...

for /R %%F in ("*.*") do (
set name=%%~nxF
set path=%%~pF

setlocal enabledelayedexpansion

if not "!name:~102,1!"=="" (

echo !path!!name! >>Results.txt
)
endlocal
)

I enable delayed expansion because working with variabls NEVER works properly in a loop without it for me. Hell, most of the time variables don't even work the way I expect them to with delayed expansion. Variable handling in batch files is incredibly convoluted.

Unsurprisingly, removing the SETLOCAL and ENDLOCAL commands and changing all the ! to % doesn't work.

I was going to try streamlining it so that it only makes one pass through the files and does all the operations at once, but first I thought I'd try adding a counter to it. Note that I haven't included a counter here because this script works as-is. I was trying to test the counter functionality by itself before trying to add it to this script.

I wanted to add separate counters for each rename operation and increment them only when a file was renamed, so that it would keep a running tally of how many filenames had been changed for each operation.

Last edited by Rekrul (17 Jul 2019 18:17)

Offline

#4 17 Jul 2019 19:11

RG
Member
From: Minnesota
Registered: 18 Feb 2010
Posts: 348

Re: I give up!!! Why doesn't this work???

I checked several of my machines and none have 'Identify.exe' on them, so I can't test your full script.

An important concept to keep in mind is the difference between 'Load Time' behavior and 'Run Time' behavior. All environment variables within a FOR loop or a parenthesized block are expanded at 'Load Time'... even if that block contains multiple lines. Therefore all environment variables will contain the value assigned to them at 'Load Time'. However, you can get around that by using CALL. In the example below I have gone back to your simplified first post to show examples of 2 counters that count the .jpg and .png files in the subroutine :Cnt. I pass "%%~xF to it as a  parameter because %%~xF is not available outside the FOR loop.

There are still some intermediate variables that are not needed... but it is ok to use them if you prefer.

echo off
set /a jpg=0
set /a png=0
for %%F in ("*.*") do (
   call :Cnt "%%~xF"
   )
echo(jpg total=%jpg%
echo(png total=%png%
pause
goto :eof

:Cnt
if "%~1"==".jpg" set /A jpg+=1
if "%~1"==".png" set /A png+=1
goto :eof

Windows Shell Scripting and InstallShield

Offline

#5 18 Jul 2019 06:20

Rekrul
Member
Registered: 17 Apr 2016
Posts: 32

Re: I give up!!! Why doesn't this work???

RG wrote:

I checked several of my machines and none have 'Identify.exe' on them, so I can't test your full script.

It's part of the Image Magick distribution;

https://imagemagick.org/index.php

There's a portable version that you can just unzip and copy Identify.exe into the directory with the script. If you don't have any images saved from Tumblr, you can fake it by just renaming some other picture files. Tumblr filenames should be in the format of;

tumblr_[letters & numbers]_[SIZE].jpg

Where [SIZE] is one of; 250, 400, 500, 540, 640, 1280. Don't ask me why they picked those numbers. If an image's width exceeds one of those numbers, the next value will be used, with 1280 being the maximum. In other words, if an image has a width of 582 pixels, it should be labeled as 640 because it's larger than 540. The filename would look something like this;

tumblr_ny5y8frbT71K9cshlo1_640.jpg

I tell you this in case you want to give my script a thorough test. Whenever a name contains the string "tumblr", it will consider the last four characters of the name to be the size indicator and will change it if it disagrees with the actual width of the image. I should probably put in some error checking in case an image contains the tumblr string, but doesn't have the size at the end...

RG wrote:

However, you can get around that by using CALL.

Unless I'm mistaken, the subroutine that you CALL will inherit the delayed expansion status of the line that called it.

RG wrote:

In the example below I have gone back to your simplified first post to show examples of 2 counters that count the .jpg and .png files in the subroutine :Cnt. I pass "%%~xF to it as a  parameter because %%~xF is not available outside the FOR loop.

Unfortunately that doesn't work. Putting the counter in a subroutine doesn't solve anything. It still has the same limitations as including it right in the main loop.

This doesn't work;

echo off
set /A c=0

for %%F in ("*.*") do (
  set extension=%%~xF

  setlocal enabledelayedexpansion

  if "!extension!"=="jpg" call count
  echo !c!

  endlocal
)

:count
set /A c=c+1
goto :eof

Yes, all this script does is count jpeg files, but I made it as simple as possible so that I could test the idea of a counter. Whenever I'm not sure how to do something, I create the simplest script I possibly can, while applying the restrictions that the finished script will need.

In the case of the full script, I need delayed expansion because of the string manipulation that it does during the loops, but delayed expansion has to be off when the filename is copied from the FOR variable into a normal variable in case the name contains "!". Which means I have to enable it after the "set name1=%%F" section and then end it before the next loop. I can't put the counter after the ENDLOCAL statement or the counter will be incremented on every loop, not just when a rename operation occurs.

Here, try this one;

echo off
echo Checking for 102+ character filenames...

for %%F in ("*.*") do (

  set name=%%~nxF

  setlocal enabledelayedexpansion

  if not "!name:~102,1!"=="" (

    echo %%F >>Results.txt
    )
  endlocal
)

Feel free to change the 102 to any arbitrary length you'd like for testing. I used 102 because that's the maximum allowed when burning files to disc. Note that "set name=%%~nxF" has to be done with delayed expansion disabled to preserve any filenames that might contain "!". It then needs to be enabled for the string operations. At least as far as I can tell. In all my tests, I couldn't get it to work properly without delayed expansion.

NOTE: The ONLY reason I tried to make it work without delayed expansion is that I was trying to come up with the simplest script I could that truly needs it, in order to demonstrate the problem.

Any counter should only be incremented IF the name is over the specified length.

Offline

#6 18 Jul 2019 14:12

bluesxman
Member
From: UK
Registered: 29 Dec 2006
Posts: 1,090

Re: I give up!!! Why doesn't this work???

I think the problem is that you aren't fully understanding the implications of adding "setlocal".

I've annotated below...

echo off
set /A c=0

for %%F in ("*.*") do (
  set extension=%%~xF
  REM this is essentially putting a box around all your variables
  REM you can change them all you like inside the box
  setlocal enabledelayedexpansion

  if "!extension!"=="jpg" call :count
  echo !c!

  REM this is closing the box and throwing it away, all variables are returned to their previous state
  REM therefore anything you have done to "c" in the meantime is lost.
  endlocal
  REM you are opening and closing the box each loop, and modifying "c" inside the box, so no
  REM changes to "c" will persist to the next loop
)

REM even though this is a sub-routine, it is still being called above within the context of a "setlocal" box,
REM so any variables are still in scope
:count
set /A c=c+1
goto :eof

for tokens are not affected by delayed expansion, so you'd have a much easier time without all the intermediate variables.
Below I have amended your simplified example using some CMD tricks and work arounds.
I think this should work, but I've not tested it:

@echo off
REM this is to ensure any existing counts are reset
for /f "usebackq tokens=1 delims==" %%a in (`set count. 2^>nul`) do set "%%~a="

for %%F in ("*.*") do (
  call :count "%%~xF"
  REM show relevant counter incrementing during the loop
  call :show "%%~xF"
)
REM display all counters at the end
call :show_all

goto :EOF
:count
REM increment a variable which has a suffix like the passed parameter
REM e.g passing ".jpg" would increment "count.jpg"
set /A count%~1+=1
goto :EOF

:show
REM using "call" and doubling % in variables is a way to get delayed expansion-like behaviour, but without enabling it
REM it has a small time penalty, that would not be significant unless you have a lot of items to process
REM here I'm trying to used the passed parameter to get a count.* value back
if defined count%~1 call echo:%~1 count: %%count%~1%%

REM I could have enabled delayed expansion for this sub routine (with a matching endlocal to close) to display the value like:
REM !count%~1! but I don't want to, because setlocal has implications (as you are discovered) so I try HARD to do without it
goto :EOF

:show_all

REM we reset any variables starting with "count." at the beginning, so this should only show our counts
for /f "usebackq tokens=2* delims==." %%a in (`set count. 2^>nul`) do echo:.%%a count: %%b
goto :EOF

Last edited by bluesxman (18 Jul 2019 14:33)


cmd | *sh | Ruby | Chef

Offline

#7 20 Jul 2019 11:29

Rekrul
Member
Registered: 17 Apr 2016
Posts: 32

Re: I give up!!! Why doesn't this work???

bluesxman wrote:

I think the problem is that you aren't fully understanding the implications of adding "setlocal".

I think the problem is that the person who designed this mess had no coherent idea of what they were doing.

bluesxman wrote:

for tokens are not affected by delayed expansion, so you'd have a much easier time without all the intermediate variables.
Below I have amended your simplified example using some CMD tricks and work arounds.
I think this should work, but I've not tested it:

Thanks, but I actually figured out a much simpler method;

echo off
echo Checking for 102+ character filenames...

set /A c=0

for %%F in ("*.*") do (

  set name=%%~nxF

  setlocal enabledelayedexpansion

  if not "!name:~102,1!"=="" (

    echo %%F >>Results.txt
    endlocal
    set /A c=c+1
    setlocal enabledelayedexpansion
    echo !c!
    )
  endlocal
)

I haven't tested it extensively yet, but it seems to work.

I'm in the process of revising my script to do everything in one pass. I'll post it when it's done. Not sure when that will be (as if everyone here is on the edge of their seat to see it) as I feel like crap right now.

Offline

#8 22 Jul 2019 07:54

bluesxman
Member
From: UK
Registered: 29 Dec 2006
Posts: 1,090

Re: I give up!!! Why doesn't this work???

If you've found a solution that works for you, cool.

FWIW it's still perfectly achievable without resorting to delayed expansion, with a couple of minor tweaks (thus saving you jumping through "setlocal/endlocal" hoops):

@echo off
echo Checking for 102+ character filenames...

set /A c=0

for %%F in ("*.*") do (
  call :sub "%%~F"
)

goto :EOF

:sub
REM you can load this sub-routine up with any other logic/checks you need to perform

set "name=%~nx1"

if "%name:~102,1%"=="" goto :EOF

echo %1 >>Results.txt
set /A c=c+1
echo %c%

goto :EOF

Last edited by bluesxman (22 Jul 2019 07:55)


cmd | *sh | Ruby | Chef

Offline

#9 29 Jul 2019 12:11

Rekrul
Member
Registered: 17 Apr 2016
Posts: 32

Re: I give up!!! Why doesn't this work???

bluesxman wrote:

If you've found a solution that works for you, cool.

Unfortunately it doesn't, which of course doesn't surprise me.

It works in the routine for finding 102 character names, but for some reason it breaks the other parts of my script.

I've tweaked my routine so that it now checks for incorrect extensions, incorrect Tumblr sizes and long filenames all in one pass. It may not be the most elegant script, but it works. Then I tried adding counters to it and completely broke it for reasons that I don't have the faintest clue about.

Again, if you want to test this script, you'll need "Identify.exe" from the Image Magick distribution.

The following script works. In all my testing, it properly renames every misnamed file;

echo off
cls

for /R %%F in ("*.*") do (

set name1=%%~nF
set name2=%%~nF
set name3=%%~nxF
set extension=%%~xF
set filepath=%%~pF
set rename=0
set pic=0
set tumblr=2

setlocal enabledelayedexpansion

if /I "!extension!"==".jpg" set pic=1
if /I "!extension!"==".jpeg" set pic=1
if /I "!extension!"==".png" set pic=1
if /I "!extension!"==".gif" set pic=1
if /I "!extension!"==".webp" set pic=1
if /I "!extension!"==".bmp" set pic=1

set test=!name1:tumblr=!

if not "!test!"=="!name1!" set tumblr=1

if "!pic!"=="1" (

   for /F %%m in ('identify -quiet -format "%%m" "%%F[0]"') do (set type=%%m)
   for /F %%w in ('identify -quiet -format "%%w" "%%F[0]"') do (set width=%%w)

if "!type!"=="JPEG" set type=.jpg
if "!type!"=="PNG" set type=.png
if "!type!"=="GIF" set type=.gif
if "!type!"=="WEBP" set type=.webp
if "!type!"=="BMP3" set type=.bmp

if /I not "!type!"=="!extension!" (
set extension=!type! & set rename=1
))

if "!pic!"=="!tumblr!" (

set size=!name1:~-4!
set size=!size:_=!

if !width! gtr 640 set width=1280
if !width! leq 250 set width=250
if !width! gtr 540 (if !width! leq 640 set width=640)
if !width! gtr 500 (if !width! leq 540 set width=540)
if !width! gtr 400 (if !width! leq 500 set width=500)
if !width! gtr 250 (if !width! leq 400 set width=400)

if not "!size!"=="!width!" (
 for %%a in (!size!) do (for %%b in (!width!) do (set name2=!name1:%%a=%%b!))
set rename=1
))

if "!rename!"=="1" (

ren "%%F" "!name2!!extension!"

if "!rename!"=="!tumblr!" (if exist "!filepath!!name1!.txt" ren "!filepath!!name1!.txt" "!name2!.txt")

)
if not "!name3:~102,1!"=="" (
echo !filepath!!name3! >>Results.txt
)
endlocal
)
pause

It sets a flag if it detects that the file is a picture according to the extension. It sets a flag if the file has "tumblr" in the name. If the pic flag is set, it uses Identify to get the type and width from the file. It sets the type to a file extension, based on the format listed in the file, then it compares it to the actual extension. If they differ, it sets the extension to the type and sets a rename flag. If both the pic and Tumblr flags are set, it then sets the width according to Tumblr rules and compares it to the size listed in the filename. If they differ, the size in the name is replaced with the width and the rename flag is set. Then if the rename flag has been set by either section, it renames the file. If both the rename flag and Tumblr flags are set, it then tries to rename a matching text file to reflect the new size in the filename. Finally, it checks for 102+ character filenames and writes them to a file if found.

The following script does NOT work. It properly reports how many files SHOULD be renamed, but the renaming never actually takes place and several of the variable operations don't work.

echo off
cls

set /A ec=0
set /A tc=0
set /A lc=0

echo.
echo    %ec% Extensions renamed
echo    %tc% Tumblr files renamed
echo    %lc% Long filenames found

for /R %%F in ("*.*") do (

set name1=%%~nF
set name2=%%~nF
set name3=%%~nxF
set extension=%%~xF
set filepath=%%~pF
set rename=0
set pic=0
set tumblr=2

setlocal enabledelayedexpansion

if /I "!extension!"==".jpg" set pic=1
if /I "!extension!"==".jpeg" set pic=1
if /I "!extension!"==".png" set pic=1
if /I "!extension!"==".gif" set pic=1
if /I "!extension!"==".webp" set pic=1
if /I "!extension!"==".bmp" set pic=1

set test=!name1:tumblr=!

if not "!test!"=="!name1!" set tumblr=1

if "!pic!"=="1" (

   for /F %%m in ('identify -quiet -format "%%m" "%%F[0]"') do (set type=%%m)
   for /F %%w in ('identify -quiet -format "%%w" "%%F[0]"') do (set width=%%w)

if "!type!"=="JPEG" set type=.jpg
if "!type!"=="PNG" set type=.png
if "!type!"=="GIF" set type=.gif
if "!type!"=="WEBP" set type=.webp
if "!type!"=="BMP3" set type=.bmp

if /I not "!type!"=="!extension!" (
set extension=!type! & set rename=1
endlocal & set /A ec=ec+1 & setlocal enabledelayedexpansion
cls
echo.
echo    !ec! Extensions renamed
echo    !tc! Tumblr files renamed
echo    !lc! Long filenames found
))

if "!pic!"=="!tumblr!" (

set size=!name1:~-4!
set size=!size:_=!

if !width! gtr 640 set width=1280
if !width! leq 250 set width=250
if !width! gtr 540 (if !width! leq 640 set width=640)
if !width! gtr 500 (if !width! leq 540 set width=540)
if !width! gtr 400 (if !width! leq 500 set width=500)
if !width! gtr 250 (if !width! leq 400 set width=400)

if not "!size!"=="!width!" (
 for %%a in (!size!) do (for %%b in (!width!) do (set name2=!name1:%%a=%%b!))
set rename=1
endlocal & set /A tc=tc+1 & setlocal enabledelayedexpansion
cls
echo.
echo    !ec! Extensions renamed
echo    !tc! Tumblr files renamed
echo    !lc! Long filenames found
))

if "!rename!"=="1" (

ren "%%F" "!name2!!extension!"

if "!rename!"=="!tumblr!" (if exist "!filepath!!name1!.txt" ren "!filepath!!name1!.txt" "!name2!.txt")

)
if not "!name3:~102,1!"=="" (
echo !filepath!!name3! >>Results.txt
endlocal & set /A lc=lc+1 & setlocal enabledelayedexpansion
cls
echo.
echo    !ec! Extensions renamed
echo    !tc! Tumblr files renamed
echo    !lc! Long filenames found
)
endlocal
)
echo.
echo.
pause

What it's supposed to do is clear the window, then print the totals at the top, updating the counters each time it detects an incorrect extension, Tumblr size or long filename. Only the long filename routine works. All the rest fail. Yet if I remove the lines that update the counters (ec, tc, lc), it works fine.

I'm guessing that by doing endlocal and then setting it again, I'm somehow clearing some, but not all the variables, but I don't know why it's doing this, how to avoid it, how to create a counter while delayed expansion is enabled, or how to carry out all the variable operations without it, since batch variable handling seems intentionally designed to frustrate even the most basic operations.

EDIT:

Just to re-interate; I don't enable delayed expansion at the start of the script because if the filename is passed to a variable with it enabled, it will strip out any "!" characters in the filename and this will cause errors when it can't find the file on the drive. I don't know how to stop this from happening other than to define the name variable before enabling delayed expansion. I enable it after those variables are defined because otherwise variables in for loops are completely broken and nothing works properly. I need it to make the variables work, but I need it off at the start to preserve the filenames. I can't define the counter variables after enabling it because then they'll be reset to zero every time it loops, but if I define them before delayed expansion is enabled, then trying to update them within the loop, it doesn't work.

Last edited by Rekrul (29 Jul 2019 23:19)

Offline

Board footer

Powered by FluxBB