Re: Fix issue #1852: manuals needs more explicit dependencies. (issue 4996044)

[reinhold . kainhofer]

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/