View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0037782 | Patches | Packages | public | 2020-09-21 01:02 | 2020-10-11 15:03 |
| Reporter | CudaText man_ | Assigned To | Bart Broersma | ||
| Priority | normal | Severity | minor | Reproducibility | N/A |
| Status | closed | Resolution | fixed | ||
| OS | ubuntu x64 | ||||
| Product Version | 2.1 (SVN) | ||||
| Target Version | 2.2 | Fixed in Version | 2.2 | ||
| Summary | 0037782: LazUtils.FileUtil.FindAllFiles patch | ||||
| Description | - added 'const' for few string params - added value for Searcher.Search parameter "CaseSensitive" (was False) as FilenamesCaseSensitive (const in FileUtil). This will make search faster on linux. | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | r63989 | ||||
| LazTarget | 2.2 | ||||
| Widgetset | |||||
| Attached Files |
| ||||
|
|
find.diff (2,259 bytes)
Index: components/lazutils/fileutil.inc
===================================================================
--- components/lazutils/fileutil.inc (revision 63900)
+++ components/lazutils/fileutil.inc (working copy)
@@ -656,23 +656,23 @@
end;
procedure FindAllFiles(AList: TStrings; const SearchPath: String;
- SearchMask: String; SearchSubDirs: Boolean; DirAttr: Word;
+ const SearchMask: String; SearchSubDirs: Boolean; DirAttr: Word;
MaskSeparator: char; PathSeparator: char);
var
Searcher: TListFileSearcher;
begin
Searcher := TListFileSearcher.Create(AList);
- Searcher.DirectoryAttribute := DirAttr;
- Searcher.MaskSeparator := MaskSeparator;
- Searcher.PathSeparator := PathSeparator;
try
- Searcher.Search(SearchPath, SearchMask, SearchSubDirs);
+ Searcher.DirectoryAttribute := DirAttr;
+ Searcher.MaskSeparator := MaskSeparator;
+ Searcher.PathSeparator := PathSeparator;
+ Searcher.Search(SearchPath, SearchMask, SearchSubDirs, FilenamesCaseSensitive);
finally
Searcher.Free;
end;
end;
-function FindAllFiles(const SearchPath: String; SearchMask: String;
+function FindAllFiles(const SearchPath: String; const SearchMask: String;
SearchSubDirs: Boolean; DirAttr: Word;
MaskSeparator: char; PathSeparator: char): TStringList;
begin
Index: components/lazutils/fileutil.pas
===================================================================
--- components/lazutils/fileutil.pas (revision 63900)
+++ components/lazutils/fileutil.pas (working copy)
@@ -180,11 +180,11 @@
constructor Create(AList: TStrings);
end;
-function FindAllFiles(const SearchPath: String; SearchMask: String = '';
+function FindAllFiles(const SearchPath: String; const SearchMask: String = '';
SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
MaskSeparator: char = ';'; PathSeparator: char = ';'): TStringList; overload;
procedure FindAllFiles(AList: TStrings; const SearchPath: String;
- SearchMask: String = ''; SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
+ const SearchMask: String = ''; SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
MaskSeparator: char = ';'; PathSeparator: char = ';'); overload;
function FindAllDirectories(const SearchPath: string;
|
|
|
Optimized TFileSearcher.Search, avoided temp string changing. find2.diff. find2.diff (3,798 bytes)
Index: components/lazutils/fileutil.inc
===================================================================
--- components/lazutils/fileutil.inc (revision 63900)
+++ components/lazutils/fileutil.inc (working copy)
@@ -656,23 +656,23 @@
end;
procedure FindAllFiles(AList: TStrings; const SearchPath: String;
- SearchMask: String; SearchSubDirs: Boolean; DirAttr: Word;
+ const SearchMask: String; SearchSubDirs: Boolean; DirAttr: Word;
MaskSeparator: char; PathSeparator: char);
var
Searcher: TListFileSearcher;
begin
Searcher := TListFileSearcher.Create(AList);
- Searcher.DirectoryAttribute := DirAttr;
- Searcher.MaskSeparator := MaskSeparator;
- Searcher.PathSeparator := PathSeparator;
try
- Searcher.Search(SearchPath, SearchMask, SearchSubDirs);
+ Searcher.DirectoryAttribute := DirAttr;
+ Searcher.MaskSeparator := MaskSeparator;
+ Searcher.PathSeparator := PathSeparator;
+ Searcher.Search(SearchPath, SearchMask, SearchSubDirs, FilenamesCaseSensitive);
finally
Searcher.Free;
end;
end;
-function FindAllFiles(const SearchPath: String; SearchMask: String;
+function FindAllFiles(const SearchPath: String; const SearchMask: String;
SearchSubDirs: Boolean; DirAttr: Word;
MaskSeparator: char; PathSeparator: char): TStringList;
begin
@@ -765,7 +765,7 @@
FSearching := False;
end;
-procedure TFileSearcher.Search(ASearchPath: String; ASearchMask: String;
+procedure TFileSearcher.Search(const ASearchPath: String; const ASearchMask: String;
ASearchSubDirs: Boolean; CaseSensitive: Boolean = False);
var
MaskList: TMaskList;
@@ -836,9 +836,9 @@
end;
var
- p: SizeInt;
+ p1, p2: Integer;
+ i: Integer;
Dir: String;
- i: Integer;
OtherDir: String;
MaskOptions: TMaskOptions;
begin
@@ -855,12 +855,14 @@
FSearching := True;
SearchDirectories:=TStringList.Create;
try
- while ASearchPath<>'' do begin
- p:=Pos(FPathSeparator,ASearchPath);
- if p<1 then
- p:=length(ASearchPath)+1;
- Dir:=ResolveDots(LeftStr(ASearchPath,p-1));
- Delete(ASearchPath,1,p);
+ p1:=1;
+ while p1<=Length(ASearchPath) do
+ begin
+ p2:=PosEx(FPathSeparator,ASearchPath,p1);
+ if p2<1 then
+ p2:=length(ASearchPath)+1;
+ Dir:=ResolveDots(Copy(ASearchPath,p1,p2-p1));
+ p1:=p2+1;
if Dir='' then continue;
Dir:=ChompPathDelim(Dir);
for i:=SearchDirectories.Count-1 downto 0 do
Index: components/lazutils/fileutil.pas
===================================================================
--- components/lazutils/fileutil.pas (revision 63900)
+++ components/lazutils/fileutil.pas (working copy)
@@ -145,7 +145,7 @@
procedure DoFileFound; virtual;
public
constructor Create;
- procedure Search(ASearchPath: String; ASearchMask: String = '';
+ procedure Search(const ASearchPath: String; const ASearchMask: String = '';
ASearchSubDirs: Boolean = True; CaseSensitive: Boolean = False);
public
property MaskSeparator: char read FMaskSeparator write FMaskSeparator;
@@ -180,11 +180,11 @@
constructor Create(AList: TStrings);
end;
-function FindAllFiles(const SearchPath: String; SearchMask: String = '';
+function FindAllFiles(const SearchPath: String; const SearchMask: String = '';
SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
MaskSeparator: char = ';'; PathSeparator: char = ';'): TStringList; overload;
procedure FindAllFiles(AList: TStrings; const SearchPath: String;
- SearchMask: String = ''; SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
+ const SearchMask: String = ''; SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
MaskSeparator: char = ';'; PathSeparator: char = ';'); overload;
function FindAllDirectories(const SearchPath: string;
|
|
|
I think in the TFileSearcher.Search function variables p1 and p2 must be of type SizeInt, not Integer. See https://www.freepascal.org/docs-html/rtl/system/length.html and https://www.freepascal.org/docs-html/rtl/system/copy.html and https://www.freepascal.org/docs-html/rtl/strutils/posex.html |
|
|
You change the default behaviour of FindAllFiles to be case-sensitive on *nix (or did I misunderstand your patch)? If so, that is not acceptable. At least it should be configurable in the call to FindAllFiles and the default behavious should remains as it is now. Also: please separate the functional changes (case-sensitive vs case-insensitive) from te optimization (const paramters). |
|
|
So, new1.diff - p1,p2: SizeInt - removed changed behaviour new1.diff (3,309 bytes)
Index: components/lazutils/fileutil.inc
===================================================================
--- components/lazutils/fileutil.inc (revision 63900)
+++ components/lazutils/fileutil.inc (working copy)
@@ -656,7 +656,7 @@
end;
procedure FindAllFiles(AList: TStrings; const SearchPath: String;
- SearchMask: String; SearchSubDirs: Boolean; DirAttr: Word;
+ const SearchMask: String; SearchSubDirs: Boolean; DirAttr: Word;
MaskSeparator: char; PathSeparator: char);
var
Searcher: TListFileSearcher;
@@ -672,7 +672,7 @@
end;
end;
-function FindAllFiles(const SearchPath: String; SearchMask: String;
+function FindAllFiles(const SearchPath: String; const SearchMask: String;
SearchSubDirs: Boolean; DirAttr: Word;
MaskSeparator: char; PathSeparator: char): TStringList;
begin
@@ -765,7 +765,7 @@
FSearching := False;
end;
-procedure TFileSearcher.Search(ASearchPath: String; ASearchMask: String;
+procedure TFileSearcher.Search(const ASearchPath: String; const ASearchMask: String;
ASearchSubDirs: Boolean; CaseSensitive: Boolean = False);
var
MaskList: TMaskList;
@@ -836,9 +836,9 @@
end;
var
- p: SizeInt;
+ p1, p2: SizeInt;
+ i: Integer;
Dir: String;
- i: Integer;
OtherDir: String;
MaskOptions: TMaskOptions;
begin
@@ -855,12 +855,14 @@
FSearching := True;
SearchDirectories:=TStringList.Create;
try
- while ASearchPath<>'' do begin
- p:=Pos(FPathSeparator,ASearchPath);
- if p<1 then
- p:=length(ASearchPath)+1;
- Dir:=ResolveDots(LeftStr(ASearchPath,p-1));
- Delete(ASearchPath,1,p);
+ p1:=1;
+ while p1<=Length(ASearchPath) do
+ begin
+ p2:=PosEx(FPathSeparator,ASearchPath,p1);
+ if p2<1 then
+ p2:=length(ASearchPath)+1;
+ Dir:=ResolveDots(Copy(ASearchPath,p1,p2-p1));
+ p1:=p2+1;
if Dir='' then continue;
Dir:=ChompPathDelim(Dir);
for i:=SearchDirectories.Count-1 downto 0 do
Index: components/lazutils/fileutil.pas
===================================================================
--- components/lazutils/fileutil.pas (revision 63900)
+++ components/lazutils/fileutil.pas (working copy)
@@ -145,7 +145,7 @@
procedure DoFileFound; virtual;
public
constructor Create;
- procedure Search(ASearchPath: String; ASearchMask: String = '';
+ procedure Search(const ASearchPath: String; const ASearchMask: String = '';
ASearchSubDirs: Boolean = True; CaseSensitive: Boolean = False);
public
property MaskSeparator: char read FMaskSeparator write FMaskSeparator;
@@ -180,11 +180,11 @@
constructor Create(AList: TStrings);
end;
-function FindAllFiles(const SearchPath: String; SearchMask: String = '';
+function FindAllFiles(const SearchPath: String; const SearchMask: String = '';
SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
MaskSeparator: char = ';'; PathSeparator: char = ';'): TStringList; overload;
procedure FindAllFiles(AList: TStrings; const SearchPath: String;
- SearchMask: String = ''; SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
+ const SearchMask: String = ''; SearchSubDirs: Boolean = True; DirAttr: Word = faDirectory;
MaskSeparator: char = ';'; PathSeparator: char = ';'); overload;
function FindAllDirectories(const SearchPath: string;
|
|
|
Applied last patch. Once we ditch fpc 3.0.4 (when 3.2.2 is released), the StrUtils isn't needed anymore, because Pos() supports the index parameter then. We could ifdef this now, or wait until fpc 3.2.2. It would be nice if you would supply a new patch then (or now with the ifdef's for the compiler version). |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-09-21 01:02 | CudaText man_ | New Issue | |
| 2020-09-21 01:02 | CudaText man_ | File Added: find.diff | |
| 2020-09-21 01:33 | CudaText man_ | Note Added: 0125692 | |
| 2020-09-21 01:33 | CudaText man_ | File Added: find2.diff | |
| 2020-09-21 02:28 | Serge Anvarov | Note Added: 0125693 | |
| 2020-09-21 12:10 | Bart Broersma | Note Added: 0125702 | |
| 2020-09-21 12:10 | Bart Broersma | Assigned To | => Bart Broersma |
| 2020-09-21 12:10 | Bart Broersma | Status | new => feedback |
| 2020-09-21 12:10 | Bart Broersma | LazTarget | => - |
| 2020-09-21 12:16 | CudaText man_ | Note Added: 0125703 | |
| 2020-09-21 12:16 | CudaText man_ | File Added: new1.diff | |
| 2020-09-21 12:16 | CudaText man_ | Status | feedback => assigned |
| 2020-10-10 11:50 | Bart Broersma | Status | assigned => resolved |
| 2020-10-10 11:50 | Bart Broersma | Resolution | open => fixed |
| 2020-10-10 11:50 | Bart Broersma | Note Added: 0126209 | |
| 2020-10-10 11:50 | Bart Broersma | Note Edited: 0126209 | View Revisions |
| 2020-10-10 11:50 | Bart Broersma | Note Edited: 0126209 | View Revisions |
| 2020-10-10 12:02 | Bart Broersma | Fixed in Version | => 2.2 |
| 2020-10-10 12:02 | Bart Broersma | Target Version | => 2.2 |
| 2020-10-10 12:02 | Bart Broersma | Fixed in Revision | => r63989 |
| 2020-10-10 12:02 | Bart Broersma | LazTarget | - => 2.2 |
| 2020-10-11 15:03 | CudaText man_ | Status | resolved => closed |
