Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Proposing a fix for the FOR /F syntax in sy[ncany].bat #270

Closed
xax opened this issue Nov 10, 2014 · 7 comments
Closed

Proposing a fix for the FOR /F syntax in sy[ncany].bat #270

xax opened this issue Nov 10, 2014 · 7 comments

Comments

@xax
Copy link

xax commented Nov 10, 2014

In the NT batch file serving as cli proxy, the line
for /F tokens^=2^,3^,5delims^=^<^"^=^> %%a in (%APP_USERCONFIG_FILE%) do (
throws an error, if the batch file is run under TCC (TakeCommand windows shell replacement by JP-Soft).

This is due to the hackish make up of the <options> string following the /F switch.
According to CMD.EXE's documentation, the string must be enclosed in double quotes (I recognize, you tried to avoid that by not using separating spaces and escaping the special characters). So this hack doesn't only break TCC compatibility, but is not valid CMD-Syntax, either.

Using the ”delayed expansion“ enhanced feature of CMD (enabled in the SETLOCAL command, if not already enabled), you can avoid the hack using a temporary environment variable to contain a single double quote character, that gets delayed-expanded as an argument to the delimiter= option of FOR /F. And that way TCC (and its users) are satisfied, too.

This is the diff:

--- sy.bat  2014-11-08 00:13:54 +0000
+++ sy.bat  2014-11-10 15:56:36 +0000
@@ -6,7 +6,7 @@
 @rem ##########################################################################

 @rem Set local scope for the variables with windows NT shell
-if "%OS%"=="Windows_NT" setlocal
+if "%OS%"=="Windows_NT" setlocal ENABLEDELAYEDEXPANSION

 @rem Add default JVM options here. You can also use JAVA_OPTS and SYNCANY_OPTS to pass JVM options to this script.
 set DEFAULT_JVM_OPTS="-Dfile.encoding=utf-8"
@@ -15,7 +15,8 @@
 set APP_USERCONFIG_FILE=%AppData%\\Syncany\\userconfig.xml

 if exist "%APP_USERCONFIG_FILE%" (
-  for /F tokens^=2^,3^,5delims^=^<^"^=^> %%a in (%APP_USERCONFIG_FILE%) do ( 
+  set QT=^"
+  for /F "tokens=2,3,5 delims=<=>!QT!" %%a in (%APP_USERCONFIG_FILE%) do (
     if "%%a" equ "maxMemory" set APP_MAX_MEMORY=%%b
   )
 )
@pimotte
Copy link
Member

pimotte commented Nov 10, 2014

Firstly, thanks for your contribution!
Secondly, I don't know anything about cmd, but it looks like you've got it under control.
Thirdly, if you care about credit in any way, you could also submit a pull-request, that way your name is tied to this change. (If you do not want to do this for any reason, we are probably fine copy/pasting, but just saying.)

@xax
Copy link
Author

xax commented Nov 10, 2014

You're welcome.

It's rather a Windows' shell issue, Microsoft's ”built-in“ CMD or a vastly compatible replacement, TakeCommand. So it's awkward batch scripting rather than bash scripting.

I've not yet reached a comprehensive glance over the repository's structure, but I assume the code I relate to be buried in the Gradle build system – which I know very little of. So I'd suggest some of the guys involved in that part of the project QA-testing my patch and applying it ”by hand“. No credits needed.

@binwiederhier
Copy link
Member

I just hate that we still have batch files. They're the worst...

For the GUI it's even worse: .lnk starts .vbs which starts a .bat which then runs the JVM. Horrible combination...

@binwiederhier
Copy link
Member

It is indeed embedded deep into gradle -- we're basically patching the batch file that gradle creates.
If you want I can quickly fix it and test it in my VM.

@cr0
Copy link
Member

cr0 commented Nov 10, 2014

wow that's indeed very ugly :D

@binwiederhier
Copy link
Member

@xax
Should be fixed in 723409f. Works on my Windows VM. Can you double check this on your machine.

You can download the latest snapshot (in 10 minutes or so) from here, once Travis is done building:
https://www.syncany.org/dist/snapshots/?C=M;O=D

@binwiederhier
Copy link
Member

Works on two Windows PCs. I now consider this fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants