[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Updates makelsr to point to lilypond exe (issue 5489134)
From: |
graham |
Subject: |
Re: Updates makelsr to point to lilypond exe (issue 5489134) |
Date: |
Mon, 02 Jan 2012 21:11:39 +0000 |
great idea! A few implementation quibbles, though.
http://codereview.appspot.com/5489134/diff/1/scripts/auxiliar/makelsr.py
File scripts/auxiliar/makelsr.py (right):
http://codereview.appspot.com/5489134/diff/1/scripts/auxiliar/makelsr.py#newcode76
scripts/auxiliar/makelsr.py:76: lilypondexe=conv_path+'lilypond'
I have a slight preference for
lilypond_bin
but this isn't a serious complaint.
http://codereview.appspot.com/5489134/diff/1/scripts/auxiliar/makelsr.py#newcode184
scripts/auxiliar/makelsr.py:184: e = os.system (lilypondexe + "
-dno-print-pages -dsafe -o /tmp/lsrtest " + dest)
1. why did you remove the -V flag?
2. AFAIK if you simply do + dest, it will break if there are any
snippets with a space in the filename. I think that as a policy we
don't allow those, but the '%s' % dest solution avoids that potential
problem. Why not keep it there?
http://codereview.appspot.com/5489134/