View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0021875LazarusLazUtilspublic2012-04-28 07:132012-04-30 19:21
ReporterTakeda Matsuki 
Assigned ToJuha Manninen 
PrioritynormalSeverityfeatureReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version1.1 (SVN)Product Build 
Target VersionFixed in Version 
Summary0021875: [Patch] Added Feature "CopyDirTree" - Copying all directory structures to target directory
Description
This patch would provide New-Feature on FileUtil, which have capability to copy all files with the structure tree from source directory to target (destination) directory.

Tested using, FPC 2.6.0a, FPC 2.6.1 svn 21072, FPC 2.7.1 svn 21031.
Demo project attached too.

Regards,
Takeda
Additional Information
Hope latest patch on ( http://bugs.freepascal.org/view.php?id=21854 [^] ), would applied too. Since, this patch would get the advantage of solution which provided on 21854.
TagsNo tags attached.
Fixed in Revisionr37072, r37109
LazTarget-
WidgetsetWin32/Win64
Attached Filespatch file icon 2012-04-28 CopyDirTree2.patch [^] (2,137 bytes) 2012-04-28 07:13 [Show Content]
7z file icon CopyDirTree-Demo.7z [^] (60,670 bytes) 2012-04-28 07:13
patch file icon 2012-04-29 CopyDirTree4.patch [^] (3,203 bytes) 2012-04-29 05:07 [Show Content]
patch file icon 2012-04-29 CopyDirTree5.patch [^] (3,226 bytes) 2012-04-29 05:28 [Show Content]
patch file icon 2012-04-29 CopyDirTree6.patch [^] (3,532 bytes) 2012-04-29 08:42 [Show Content]
patch file icon 2012-04-29 CopyDirTree7.patch [^] (3,535 bytes) 2012-04-29 09:38 [Show Content]
patch file icon 2012-04-29 CopyDirTree8.patch [^] (564 bytes) 2012-04-29 18:47 [Show Content]
patch file icon 2012-04-29 CopyDirTree9.patch [^] (2,233 bytes) 2012-04-29 21:10 [Show Content]
patch file icon 2012-04-30 Another Idea CopyDirTree10 (Meet with CopyFile).patch [^] (2,767 bytes) 2012-04-30 07:02 [Show Content]
patch file icon 2012-04-30 Another Idea CopyDirTree10 (Meet with CopyFile)2.patch [^] (2,770 bytes) 2012-04-30 07:09 [Show Content]

- Relationships

-  Notes
(0059065)
Juha Manninen (developer)
2012-04-28 16:13
edited on: 2012-04-28 16:15

Please derive a class from TFileSearcher like FindAllFiles does, instead of using FindAllFiles directly. Reading to a temporary list can have a memory / speed impact on very big directory structures.
TFileSearcher also has DoDirectoryFound which you can use for creating directories, instead of calling ExtractFilePath. (You call it twise!).

The idea is good and I will apply it once it is improved.

(0059080)
Takeda Matsuki (reporter)
2012-04-29 05:12

As your requested, I created "TCopyDirTree" instead direct calling "FindAllFiles". In the CopyDirTree I also use the new features of "CopyFile". Seem, the new CopyFile working properly.


Regards,
Takeda.
(0059087)
Takeda Matsuki (reporter)
2012-04-29 09:44

On Latest Patch -> "2012-04-29 CopyDirTree7.patch", it was fixed the issue for "incomplete-copy into target directory" when source directory have multiple children directories (on tree) which not contains files.

-takeda-
(0059090)
Juha Manninen (developer)
2012-04-29 11:06

I modified your patch heavily and applied it.
Thanks.

Some notes about your code:
- there is much useless code. Please look at my modified version which does the exact same thing but is more compact and optimized.

- you test PreserveTime at every iteration and call CopyFile with little different params. A better way is to calculate the needed flags for CopyFile once. See the modified code.

- rfReplaceAll parameter for StringReplace is wrong. You want to replace the path only once! See the modified code.

- you implement DoDirectoryFound to create directories with ForceDirectoriesUTF8, but you also call CopyFile with flag cffCreateDestDirectory. You should select only one way of creating directories. I selected CopyFile with cffCreateDestDirectory.

- your code has different style (indentation etc.) than the surrounding code. Lazarus sources have different styles but changes should always follow the style of surrounding code. You use also indentation of one char which is not used anywhere else.

BTW: You can do this:
  Result:=SomeFunc;
instead of this:
  if SomeFunc then Result:=True
  else Result:=False;

My only worry is if StringReplace works right in all situations. I didn't find problems in my tests.

After my critical comments, the fact is that your code worked.
After some practice you will be a top-notch developer. :)
(0059093)
Juha Manninen (developer)
2012-04-29 11:35

Ok, I didn't see your version "7" before committing.
True, empty directories are not handled correctly.
I added an empty DoDirectoryFound at r37074. Please create one more patch against it.

P.S.
  Test your changes before uploading patches. You have uploaded every intermediate version.
(0059104)
Takeda Matsuki (reporter)
2012-04-29 18:47
edited on: 2012-04-29 18:51

The sound is good. I love it.
Thank you so much Mr. Juha. I so happy to know you, Sir. \(^_^)/

Well, for "DoDirectoryFound" I still prefer to use "ForceDirectoriesUTF8" instead "CopyFile".

1. coz in my tests, "CopyFile" was failed to copying the "directory" (e.g. "C:\AllTest"
   To "D:\HelloTest"). I investigate the source, it caused by "ExtractFilePath" inside
      "(not ForceDirectoriesUTF8(ExtractFilePath(DestFileName)))" located on :

   " if (cffCreateDestDirectory in Flags)
      and (not DirectoryExistsUTF8(ExtractFilePath(DestFileName)))
      and (not ForceDirectoriesUTF8(ExtractFilePath(DestFileName))) then "

   The reason why I use "ForceDirectoriesUTF8" on "DoDirectoryFound" (my patch
   version7) is caused in my tests, "CopyFile" was failed to work to copying the
   directory. I thought, "CopyFile" was perfect to copying the file not "copying
   directory".

2. I test ExtractFilePath, it seem ExtractFilePath would cutting the path :

   e.g. : "C:\Users\Takeda\Documents\New folder (2)ad อยู่ห่างๆอย่างห่วง ๆ"

   Would be "C:\Users\Takeda\Documents\"

3. (@_@), Do we need "TFileStreamUTF8" as temporary "cached" location to create
   Directory? (in case for "DoDirectoryFound"), whether it is appropriate to create
   Directory by using "TFileStreamUTF8"? <- It causing some error (and crashed), If
   I tried to Copying the directory (not the file).

If we still want to use "CopyFile" instead "ForceDirectoriesUTF8" on "DoDirectoryFound", then we must creating the trap which investigating DestFileName is Directory or not (not physically checking, coz it would failed) on "CopyFile".

For while, we can use "ForceDirectoriesUTF8" on "DoDirectoryFound", like I did on my patch file (2012-04-29 - CopyDirTree8.patch). I would tried to get solution to make "CopyFile" would capable to Copying the directory "only" (if user want to Copy Directory not file).

Hmm, Well, I already test my all my patches, before I uploaded to here, (Fact I always tired all the possibilities which "might" would drive the crash and error on runtime). But I don't know why some bugs always come to me. I thought it caused Takeda is Human, and it's normal. LOL. :))


-Takeda-

(0059110)
Takeda Matsuki (reporter)
2012-04-29 21:09

I think, it would be better if we have control for "overwrite", so on "2012-04-29 - CopyDirTree9.patch" I added it.

Also, in "2012-04-29 - CopyDirTree9.patch" I fixed my fault for "DoDirectoryFound" on the "2012-04-29 - CopyDirTree8.patch". Sorry for many patch files. Hope you (Mr. Juha) and another developers not angry to me.

I don't know "How to express my feeling related about Lazarus as RAD", Fact I really love Lazarus so much. :)

Thank you so much for Mr. Juha and another developers of Lazarus.
Have a nice day.

-Takeda-
(0059124)
Takeda Matsuki (reporter)
2012-04-30 07:01
edited on: 2012-04-30 07:11

Well, this morning, I got solution to meet with your request to use CopyFile rather than "ForceDirectoriesUTF8" directly.

I adding "cffNoWriteFileMode" inside CopyFileFlag. It would tell to "CopyFile", we would not use TFileStream. So there is no Error nor crash at runtime (in my test).

Like "2012-04-29 - CopyDirTree9.patch", I adding the features to make us get control to decide "overwrite" or not. ;)

Please review, my "2012-04-30 Another Idea CopyDirTree10 (Meet with CopyFile).Patch" file.

Regards,
Takeda.


EDIT : Please review, my "2012-04-30 Another Idea CopyDirTree10 (Meet with CopyFile)2.Patch" file. -> I forgot to add "else", coz today it's my deadline to send my app into client. :(

(0059133)
Juha Manninen (developer)
2012-04-30 19:21

Using ForceDirectoriesUTF8 in DoDirectoryFound is perfect. Maybe you misunderstood my comments.
I used the same TCopyFileFlags also for CopyDirTree. Please test.

- Issue History
Date Modified Username Field Change
2012-04-28 07:13 Takeda Matsuki New Issue
2012-04-28 07:13 Takeda Matsuki File Added: 2012-04-28 CopyDirTree2.patch
2012-04-28 07:13 Takeda Matsuki Widgetset => Win32/Win64
2012-04-28 07:13 Takeda Matsuki File Added: CopyDirTree-Demo.7z
2012-04-28 15:23 Juha Manninen Status new => assigned
2012-04-28 15:23 Juha Manninen Assigned To => Juha Manninen
2012-04-28 16:13 Juha Manninen LazTarget => -
2012-04-28 16:13 Juha Manninen Note Added: 0059065
2012-04-28 16:13 Juha Manninen Status assigned => feedback
2012-04-28 16:15 Juha Manninen Note Edited: 0059065
2012-04-29 05:07 Takeda Matsuki File Added: 2012-04-29 CopyDirTree4.patch
2012-04-29 05:12 Takeda Matsuki Note Added: 0059080
2012-04-29 05:28 Takeda Matsuki File Added: 2012-04-29 CopyDirTree5.patch
2012-04-29 08:42 Takeda Matsuki File Added: 2012-04-29 CopyDirTree6.patch
2012-04-29 09:38 Takeda Matsuki File Added: 2012-04-29 CopyDirTree7.patch
2012-04-29 09:44 Takeda Matsuki Note Added: 0059087
2012-04-29 11:06 Juha Manninen Fixed in Revision => r37072
2012-04-29 11:06 Juha Manninen Status feedback => resolved
2012-04-29 11:06 Juha Manninen Resolution open => fixed
2012-04-29 11:06 Juha Manninen Note Added: 0059090
2012-04-29 11:35 Juha Manninen Status resolved => assigned
2012-04-29 11:35 Juha Manninen Resolution fixed => reopened
2012-04-29 11:35 Juha Manninen Note Added: 0059093
2012-04-29 18:47 Takeda Matsuki Note Added: 0059104
2012-04-29 18:47 Takeda Matsuki File Added: 2012-04-29 CopyDirTree8.patch
2012-04-29 18:51 Takeda Matsuki Note Edited: 0059104
2012-04-29 21:09 Takeda Matsuki Note Added: 0059110
2012-04-29 21:10 Takeda Matsuki File Added: 2012-04-29 CopyDirTree9.patch
2012-04-30 07:01 Takeda Matsuki Note Added: 0059124
2012-04-30 07:02 Takeda Matsuki File Added: 2012-04-30 Another Idea CopyDirTree10 (Meet with CopyFile).patch
2012-04-30 07:02 Takeda Matsuki Note Edited: 0059124
2012-04-30 07:09 Takeda Matsuki File Added: 2012-04-30 Another Idea CopyDirTree10 (Meet with CopyFile)2.patch
2012-04-30 07:11 Takeda Matsuki Note Edited: 0059124
2012-04-30 19:21 Juha Manninen Fixed in Revision r37072 => r37072, r37109
2012-04-30 19:21 Juha Manninen Status assigned => resolved
2012-04-30 19:21 Juha Manninen Resolution reopened => fixed
2012-04-30 19:21 Juha Manninen Note Added: 0059133



MantisBT 1.2.12[^]
Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker