LGTM, although I would also move that code changing global_options.output_dir from do_file to main, to make things much clearer!
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py File scripts/lilypond-book.py (left): http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691 scripts/lilypond-book.py:691: + '.%s' % global_options.format) On 2011/09/09 19:19:47, janek wrote:
Pardon my ignorance, but what the '.%s' % part was doing before?
It simply prepended the "." and the correct file extension to the base name (which is the file name without extension). Granted, writing it as base_file_name + "." + global_options.format would have given the same result... http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py File scripts/lilypond-book.py (right): http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode539 scripts/lilypond-book.py:539: global_options.output_dir = os.path.abspath(global_options.output_dir) Ouch, that's the real culprit here: Why do we change the GLOBAL options in the function to process one file? That code should be moved out of do_file and into main, so the option handling is easier to understand! http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode677 scripts/lilypond-book.py:677: relative_output_dir = global_options.output_dir On 2011/09/09 19:28:29, Graham Percival wrote:
did you want an os.path.relpath() here or something? I don't see the point of defining
relative_output_dir
instead of just keeping global_options.output_dir down on line 690.
The Problem is that do_file above CHANGES the global_options.output_dir to an absolute path... It took me a while to figure that out, too. http://codereview.appspot.com/4996044/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
