On 10/21/2009 11:12 PM, Uwe Stöhr wrote:
> This looks like it should work, but I haven't tested it. I don't
have anything at the moment that > would use the new functionality.
The functionality is needed to be able to use external converters like
Alex's eLyXer script. I therefore wait for Alex's test result before
applying.
> That said, are we sure we always want to create all the directories
that might happen to be there? > Might we check first to see if there
are any files that need copying, and only then create the
> directory?
You are right. In case there is a subdirectory "images" that only
contain JPEG images but the script should only copy PNG files, we
create an empty folder. As you said this doesn't harm but can easily
be avoided as done in the attached patch.
While implementing this, I found another bug in the script: The given
list of file extensions was ignored because the "exts" list wasn't
given to the "copy_all" routine.
Attached is a better patch for branch that fixes both issues.
A few comments.
First, create_dir should return a boolean, telling us whether the
creation succeeded. Otherwise, you try to copy the file after a failure.
So I'd do:
+def create_dir(new_dir):
+ "Try to create the output directory if it doesn't exist"
+ if not os.path.isdir(new_dir):
+ try:
+ os.makedirs(new_dir)
+ except:
+ error("Unable to create %s" % new_dir)
+ return false
+ return true
+
I know I originally wrote this (copying something someone else did), but
maybe we should have more explicit error messages here. Can we tell the
user why the creation failed?
Then in here:
+def copy_all(from_dir, to_dir, exts):
+ "Copy all matching files in from_dir to to_dir"
for file in os.listdir(from_dir):
- if os.path.isdir(file):
+ if os.path.isdir(os.path.join(from_dir, file)):
+ copy_all(os.path.join(from_dir, file), os.path.join(to_dir, file),
exts)
continue
junk, ext = os.path.splitext(os.path.basename(file))
ext = ext.lower()[1:] #strip the leading dot
- try:
- # if exts is empty we ignore it
- # otherwise check if the extension is in the list
- not exts or exts.index(ext)
- except:
- continue #not found
+ # only create a directory and copy files when either
+ # exts is empty or when ext is in the exts list
+ if (exts) and (ext not in exts):
+ return
This isn't right. Suppose the directory has a.jpg and b.png, and we are
supposed to copy the pngs. Then we return as soon as we see a.jpg, and
nothing gets copied. I think a "continue" here would work, though.
+ create_dir(to_dir)
So here we need:
if not create_dir(to_dir):
return 1
I think we should return now, since we know we've failed. And we need to
return a flag saying whether we have succeeded.
from_file = os.path.join(from_dir, file)
- to_file = os.path.join(to_dir, file)
+ to_file = os.path.join(to_dir, file)
shutil.copyfile(from_file, to_file)
try:
shutil.copymode(from_file, to_file)
except:
pass
- return 0
Keep the return value here, as noted above.
Then in main:
@@ -60,35 +57,37 @@
if not os.path.isabs(to_dir):
error("%s is not an absolute file name.\n%s" % to_dir, usage(progname))
- # try to create the output directory if it doesn't exist
- if not os.path.isdir(to_dir):
- try:
- os.makedirs(to_dir)
- except:
- error("Unable to create %s" % to_dir)
+ copy_all(from_dir, to_dir, exts)
+ return 0
instead:
return copy_all(from_dir, to_dir, exts)
Then LyX will know if we fail.
rh
