View Issue Details

IDProjectCategoryView StatusLast Update
0034476LazarusPatchpublic2018-10-30 22:25
ReporterPascal RiekenbergAssigned ToJuha Manninen 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindows 10 x64OS Version1803
Product Version2.1 (SVN)Product Build59380 
Target VersionFixed in Version 
Summary0034476: SwitchPathDelims also switches options in Windows
DescriptionSwitchPathDelims is used in TCompilationToolOptions.LoadFromXML for
"Command/Value" options (Project options/Compiler Options/Compiler Commands).

If you have a command like "cmd /C \\server\path\executable.exe" the option "/C" is also switched.

Attached patch fixes this, but maybe SwitchPathDelims is not the right place as
it should only receive filenames and not commands!
TagsNo tags attached.
Fixed in Revisionr59395, r59396
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • lazfileutils.pas.patch (886 bytes)
    Index: components/lazutils/lazfileutils.pas
    ===================================================================
    --- components/lazutils/lazfileutils.pas	(revision 59380)
    +++ components/lazutils/lazfileutils.pas	(working copy)
    @@ -811,7 +811,7 @@
     
     function SwitchPathDelims(const Filename: string; Switch: TPathDelimSwitch): string;
     var
    -  i: Integer;
    +  i, l: Integer;
       p: Char;
     begin
       Result:=Filename;
    @@ -821,8 +821,17 @@
       pdsWindows: p:='\';
       else exit;
       end;
    -  for i:=1 to length(Result) do
    -    if Result[i] in ['/','\'] then
    +  l := length(Result);
    +  for i:=1 to l do
    +    if (Result[i] in ['/','\'])
    +    { ignore options like in "cmd /C \\server\path\executable.exe" }
    +    and (
    +      (p = '/')
    +      or (i = 1)
    +      or (i + 2 > l)
    +      or (Result[i - 1] <> ' ')
    +      or not (Result[i + 2] in [' ', ':'])
    +    ) then
           Result[i]:=p;
     end;
     
    
  • compileroptions.pp.patch (2,038 bytes)
    Index: ide/compileroptions.pp
    ===================================================================
    --- ide/compileroptions.pp	(revision 59394)
    +++ ide/compileroptions.pp	(working copy)
    @@ -4274,10 +4274,50 @@
     
     procedure TCompilationToolOptions.LoadFromXMLConfig(XMLConfig: TXMLConfig;
       const Path: string; DoSwitchPathDelims: boolean);
    +var
    +  Params: TStrings;
    +  param, cmd: String;
    +  p, p2, i, j: Integer;
     begin
       //debugln(['TCompilationToolOptions.LoadFromXMLConfig ',Command,' Path=',Path,' DoSwitchPathDelims=',DoSwitchPathDelims]);
    -  Command:=SwitchPathDelims(XMLConfig.GetValue(Path+'Command/Value',''),
    -                            DoSwitchPathDelims);
    +  Command:=XMLConfig.GetValue(Path+'Command/Value','');
    +  if DoSwitchPathDelims then begin
    +    if (Command<>'')
    +    and (PathDelim='\') then begin
    +      // specialhandling on windows to not switch path delimiters in options
    +      Params:=TStringList.Create;
    +      try
    +        SplitCmdLineParams(Command,Params);
    +        cmd:=SwitchPathDelims(Params[0],True);
    +        for i:=1 to Params.Count-1 do begin
    +          param:=Params[i];
    +          p:=-1;
    +          p2:=-1;
    +          for j:=1 to length(param) do
    +            if p>1 then
    +              break
    +            else if param[j]='/' then
    +              p:=j
    +            else if param[j]=':' then
    +              p2:=j;
    +          if p=1 then
    +            // param is option (the only / is at pos 1)
    +            if p2<>-1 then
    +              // potential filename after colon in option
    +              cmd+=' '+copy(param,1,p2)+SwitchPathDelims(Copy(param,p2+1,length(param)-p2),True)
    +            else
    +              cmd+=' '+param
    +          else
    +            cmd+=' '+SwitchPathDelims(param,True);
    +        end;
    +        Command:=cmd;
    +      finally
    +        Params.Free;
    +      end;
    +    end else begin
    +      Command:=SwitchPathDelims(Command,DoSwitchPathDelims);
    +    end;
    +  end;
       LoadStringList(XMLConfig,Parsers,Path+'Parsers/');
       if Parsers.Count=0 then begin
         // read old format
    
    compileroptions.pp.patch (2,038 bytes)

Activities

Pascal Riekenberg

2018-10-29 13:14

reporter  

lazfileutils.pas.patch (886 bytes)
Index: components/lazutils/lazfileutils.pas
===================================================================
--- components/lazutils/lazfileutils.pas	(revision 59380)
+++ components/lazutils/lazfileutils.pas	(working copy)
@@ -811,7 +811,7 @@
 
 function SwitchPathDelims(const Filename: string; Switch: TPathDelimSwitch): string;
 var
-  i: Integer;
+  i, l: Integer;
   p: Char;
 begin
   Result:=Filename;
@@ -821,8 +821,17 @@
   pdsWindows: p:='\';
   else exit;
   end;
-  for i:=1 to length(Result) do
-    if Result[i] in ['/','\'] then
+  l := length(Result);
+  for i:=1 to l do
+    if (Result[i] in ['/','\'])
+    { ignore options like in "cmd /C \\server\path\executable.exe" }
+    and (
+      (p = '/')
+      or (i = 1)
+      or (i + 2 > l)
+      or (Result[i - 1] <> ' ')
+      or not (Result[i + 2] in [' ', ':'])
+    ) then
       Result[i]:=p;
 end;
 

Mattias Gaertner

2018-10-29 13:33

manager   ~0111653

SwitchPathDelims is only called when opening under Linux a lpi file created under Windows. It switches \ and /.
When you open the lpi file again under Windows SwitchPathDelims switch \ and / again, restoring the path.

Mattias Gaertner

2018-10-29 13:35

manager   ~0111654

Sorry, that was wrong. I confused it with another IDE feature.

Juha Manninen

2018-10-29 14:51

developer   ~0111658

> ... maybe SwitchPathDelims is not the right place as it should only receive filenames and not commands!

Exactly! I don't know why you made such a hack.

Pascal Riekenberg

2018-10-29 15:04

reporter   ~0111659

>> ... maybe SwitchPathDelims is not the right place as it should only receive >filenames and not commands!
>
>Exactly! I don't know why you made such a hack.

Because this function was used!

Juha Manninen

2018-10-29 19:40

developer   ~0111662

Yes but the command must be parsed in TCompilationToolOptions.LoadFromXMLConfig and path delimiters switched only in the filename.
There is code somewhere for parsing commands. Mattias may remember by hart.
It is not difficult to implement from scratch either.

Pascal Riekenberg

2018-10-30 05:45

reporter   ~0111669

Okay, i'll do that.

Juha Manninen

2018-10-30 19:54

developer   ~0111691

Please test with r59395.
It solves this particular case but I guess it must be improved for other cases.

Pascal Riekenberg

2018-10-30 20:39

reporter   ~0111692

Last edited: 2018-10-30 20:40

View 3 revisions

I wonder why you solved it yourself as i said that i'll do it?
See attached patch.

Pascal Riekenberg

2018-10-30 20:39

reporter  

compileroptions.pp.patch (2,038 bytes)
Index: ide/compileroptions.pp
===================================================================
--- ide/compileroptions.pp	(revision 59394)
+++ ide/compileroptions.pp	(working copy)
@@ -4274,10 +4274,50 @@
 
 procedure TCompilationToolOptions.LoadFromXMLConfig(XMLConfig: TXMLConfig;
   const Path: string; DoSwitchPathDelims: boolean);
+var
+  Params: TStrings;
+  param, cmd: String;
+  p, p2, i, j: Integer;
 begin
   //debugln(['TCompilationToolOptions.LoadFromXMLConfig ',Command,' Path=',Path,' DoSwitchPathDelims=',DoSwitchPathDelims]);
-  Command:=SwitchPathDelims(XMLConfig.GetValue(Path+'Command/Value',''),
-                            DoSwitchPathDelims);
+  Command:=XMLConfig.GetValue(Path+'Command/Value','');
+  if DoSwitchPathDelims then begin
+    if (Command<>'')
+    and (PathDelim='\') then begin
+      // specialhandling on windows to not switch path delimiters in options
+      Params:=TStringList.Create;
+      try
+        SplitCmdLineParams(Command,Params);
+        cmd:=SwitchPathDelims(Params[0],True);
+        for i:=1 to Params.Count-1 do begin
+          param:=Params[i];
+          p:=-1;
+          p2:=-1;
+          for j:=1 to length(param) do
+            if p>1 then
+              break
+            else if param[j]='/' then
+              p:=j
+            else if param[j]=':' then
+              p2:=j;
+          if p=1 then
+            // param is option (the only / is at pos 1)
+            if p2<>-1 then
+              // potential filename after colon in option
+              cmd+=' '+copy(param,1,p2)+SwitchPathDelims(Copy(param,p2+1,length(param)-p2),True)
+            else
+              cmd+=' '+param
+          else
+            cmd+=' '+SwitchPathDelims(param,True);
+        end;
+        Command:=cmd;
+      finally
+        Params.Free;
+      end;
+    end else begin
+      Command:=SwitchPathDelims(Command,DoSwitchPathDelims);
+    end;
+  end;
   LoadStringList(XMLConfig,Parsers,Path+'Parsers/');
   if Parsers.Count=0 then begin
     // read old format
compileroptions.pp.patch (2,038 bytes)

Juha Manninen

2018-10-30 22:10

developer   ~0111695

I also found the function SplitCmdLineParams. I knew my version was not perfect, I believe yours is better.
Applied in r59396. Thanks.

Pascal Riekenberg

2018-10-30 22:25

reporter   ~0111696

Okay. But you could have saved some time as we both did the same work now.

Issue History

Date Modified Username Field Change
2018-10-29 13:14 Pascal Riekenberg New Issue
2018-10-29 13:14 Pascal Riekenberg File Added: lazfileutils.pas.patch
2018-10-29 13:33 Mattias Gaertner Note Added: 0111653
2018-10-29 13:35 Mattias Gaertner Note Added: 0111654
2018-10-29 14:51 Juha Manninen Note Added: 0111658
2018-10-29 15:04 Pascal Riekenberg Note Added: 0111659
2018-10-29 19:40 Juha Manninen Note Added: 0111662
2018-10-30 05:45 Pascal Riekenberg Note Added: 0111669
2018-10-30 19:20 Juha Manninen Assigned To => Juha Manninen
2018-10-30 19:20 Juha Manninen Status new => assigned
2018-10-30 19:54 Juha Manninen LazTarget => -
2018-10-30 19:54 Juha Manninen Note Added: 0111691
2018-10-30 19:54 Juha Manninen Status assigned => feedback
2018-10-30 20:39 Pascal Riekenberg Note Added: 0111692
2018-10-30 20:39 Pascal Riekenberg Status feedback => assigned
2018-10-30 20:39 Pascal Riekenberg File Added: compileroptions.pp.patch
2018-10-30 20:39 Pascal Riekenberg Note Edited: 0111692 View Revisions
2018-10-30 20:40 Pascal Riekenberg Note Edited: 0111692 View Revisions
2018-10-30 22:10 Juha Manninen Fixed in Revision => r59395, r59396
2018-10-30 22:10 Juha Manninen Note Added: 0111695
2018-10-30 22:10 Juha Manninen Status assigned => resolved
2018-10-30 22:10 Juha Manninen Resolution open => fixed
2018-10-30 22:25 Pascal Riekenberg Note Added: 0111696
2018-10-30 22:25 Pascal Riekenberg Status resolved => closed