Menu
▾
▴
#2343 Faulty file-naming when outputting multiple \books
Originally created by: *anonymous
Originally created by: ma...@gregoriana.sk
Originally owned by: dak@gnu.org
Reported by Michael Hendry:
http://lists.gnu.org/archive/html/bug-lilypond/2012-02/msg00951.html
Following code produces files named alto.pdf, alto-1.pdf, alto-2.pdf, alto-3.pdf, while avaiting guitar.pdf, concert.pdf, trumpet.pdf, alto.pdf:
\version "2.15.30"
music = \repeat unfold 8 { a }
GuitarBook =
\book {
\bookOutputName "guitar"
\new Score { \music }
}
ConcertBook =
\book {
\bookOutputName "concert"
\new Score { \music }
}
TrumpetBook =
\book {
\bookOutputName "trumpet"
\new Score { \music }
}
AltoBook =
\book {
\bookOutputName "alto"
\new Score { \music }
}
\book { \AltoBook }
\book { \GuitarBook }
\book { \ConcertBook }
\book { \TrumpetBook }
Read the original message for the background.
Discussion
Originally posted by: Elu...@gmail.com
- this is not a minimal example!
- where do you find the evidence that assigning a whole \book, \layout, \paper, \score etc. to a variable should work? (of course this would be nice…)
Status: Invalid
Originally posted by: dak@gnu.org
I consider the example pretty minimal. One could likely reduce it to three books. So what? That's more important for the regtest than the initial error report. The report does not contain more constructs than required, and only marginally more than probably minimal.
The evidence that assigning a whole \book, \layout, \paper, \score etc. to a variable should work is in the notation reference, in the LilyPond grammar appendix.
It does work, and that is nice. However, output file names are not associated with the book definitions but made globally, and that is not nice and it is inconsistent. I am currently looking where they should best be kept, and the paper definition might be a useful place.
Labels: -Type-Ugly Type-Defect
Owner: dak@gnu.org
Status: Accepted
Originally posted by: ianhuli...@gmail.com
I just want to add some context to this issue, and maybe save David some time investigating.
The idea is any \book block is equivalent (sort of) to an output file, which is used for all the backend filenames (pdf, midi whatever), and the \bookOutputName and \bookOutputSuffix work with this in mind. Internally the C++ code adds an implicit \book declaration at top-level even if the source code does not use \book.
\bookOutputName and \bookOutputSuffix use a parser variable to store the values.
I suspect that the problem here may not be assigning the \book declaration to a variable, but that the code from the OP is effectively nesting \book declarations, e.g.
AltoBook =
\book {
\bookOutputName "alto"
\new Score { \music }
}
\book { \AltoBook }
I further suspect that doing this is itself working round a restriction, as coding
\AltoBook without the enclosing \book will cause the parser to throw a hissy fit.
I would steer clear of making facilities like \bookOutputName and ...suffix part of \paper or \layout because they are properties of an instance of a compilation session, and not a particular piece of output. For example, \bookOutputName affects the resultant name of a midi file, why should this be controlled by a declaration of in \paper or \layout?
The original suggestion was for these functions to be property settings, but I couldn't find a home for them as the prevailing opinion was that properties had to be related in some way to contexts which implied some sort of link to the music translation process. Output file-names etc. are a more general beast so I implemented them as functions rather than properties.
I think you have two related issues here
1) \bookOutputName and \bookOutputSuffix in an outer \book block supersede definitions from an inner \book block.
2) needing to nest \book blocks is a work-round to allow the parser to accept \book definitions in a variable. Are there any black magic incantations we could invoke in the parser to ease the restriction? Conceptually, having to nest \book blocks is a bit of a nonsense, bearing in mind \book's relation to output files.
Hope this helps,
Cheers,
Ian Hulin
Labels: Needs-design
Originally posted by: dak@gnu.org
As you can easily see when looking at the definition of \bookOutputName and \bookOutputSuffix, they don't use a parser variable but rather a global variable. There is no nesting of \book blocks here, like \context { \Score ... } is not a nesting of context definitions. It is just a syntax for referring to an existing \book definition, and you can't use a book variable anywhere but at the top of a \book block.
The whole point of the original poster was that the outputname can be changed in midsession. So it is perfectly sensible to store it in a book definition. The question is just where. The \paper output definition makes sense to me. That is not an absolute: I am certainly open to better suggestions.
A global variable does not make sense to me. That's what we have now.
Originally posted by: dak@gnu.org
Issue 2343: Faulty file-naming when outputting multiple \books
http://codereview.appspot.com/5698050
Labels: Patch-new
Originally posted by: dak@gnu.org
Patchy the autobot says: LGTM.
Labels: Patch-review
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-review Patch-countdown
Originally posted by: ianhuli...@gmail.com
Just a thought, Before I implemented \bookOutputSuffix, the only way of controlling the version suffix was to set a Scheme variable output-suffix, e.g.
"output-suffix" = "Allegro"
\book {
\paper {
}
...
}
"output-suffix" = "Andante"
\book{
\paper {
}
'''
}
"output-suffix"= "Alleluia"
\book {
\paper {
}
...
}
So we retained the global variable for this and the new \bookOutputName function to retain backwards compatibility.
The above sort of fail gives this output pre this patch:
ian@nanny-ogg ~/Dropbox/Development/Lilypondwork/OutputFilenameTests$ $LILYBUILD/out/bin/lilypond Exsultate-Osuffix.ly
GNU LilyPond 2.15.30
Processing `Exsultate-Osuffix.ly'
Parsing...
Interpreting music...
Interpreting music...
Exsultate-Osuffix.ly:67:51: warning: Two simultaneous tempo-change events, junking this one
\transposition f \time 4/4
\tempo "Allegro" \hornsmovoneMusic
Exsultate-Osuffix.ly:21:1: warning: Previous tempo-change event here
\tempo "Allegro" 4=128
Preprocessing graphical objects...
MIDI output to `Exsultate-Osuffix-Allegro.midi'...
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `Exsultate-Osuffix-Allegro.ps'...
Converting to `./Exsultate-Osuffix-Allegro.pdf'...
Interpreting music...
Interpreting music...
Exsultate-Osuffix.ly:163:61: warning: Two simultaneous tempo-change events, junking this one
\transposition f \time 4/4 \key e \major
\tempo "Allegro" \hornsmovtwoMusic
Exsultate-Osuffix.ly:111:1: warning: Previous tempo-change event here
\tempo "Andante" 4=72
Preprocessing graphical objects...
MIDI output to `Exsultate-Osuffix-Andante.midi'...
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `Exsultate-Osuffix-Andante.ps'...
Converting to `./Exsultate-Osuffix-Andante.pdf'...
Interpreting music...
Interpreting music...
Exsultate-Osuffix.ly:224:5: warning: already have slur
d8
( c) bf-. a-. |
Preprocessing graphical objects...
MIDI output to `Exsultate-Osuffix-Alleluia.midi'...
Finding the ideal number of pages...
Fitting music on 1 page...
Drawing systems...
Layout output to `Exsultate-Osuffix-Alleluia.ps'...
Converting to `./Exsultate-Osuffix-Alleluia.pdf'...
Success: compilation successfully completed
ian@nanny-ogg ~/Dropbox/Development/Lilypondwork/OutputFilenameTests$
I reckon this patch is probably going to deprecate this sort of usage, so we may need something in CHANGES or whatever, or have a convert-ly rule to change
"output-suffix" = <blah>
or #(define output-suffix <blah>) to
\bookOutputSuffix <blah>
I found the directory with all the test files I had for playing with this stuff. Unfortunately I never figured out a way of getting the regression test suite to process this stuff as they tend to rely on fixed input and output file names.
However, if you'd find it helpful, I'll comb through all the junk I have and get together a list of Test conditions so you can put this patch through the wringer.
Cheers,
Ian
Originally posted by: dak@gnu.org
I'll see what I can come up with in the context of convert-ly. It would be conceivable to let "paper-variable" consult a global/parser variable as last fallback: that would likely be most transparent and compatible. But not necessarily most pretty. I think I prefer leaving this to convert-ly, even if the compatibility is not quite as good.
Originally posted by: dak@gnu.org
Ian, the patch does not change the use of "output-suffix". As long as you don't declare a book specific suffix with \bookOutputSuffix (or an assignment to book-outputsuffix or book-filename in a \paper block), the old usage will work as before.
Would it make sense to use the paper variables output-suffix and, uh, output-filename instead? Looks redundant to have "book-" in the variable name, and, after all, there seems little wrong with just declaring
\book { \paper { outputsuffix = "xxx" } }
Originally posted by: ColinPKC...@gmail.com
Counted down to 20120226, please push. If David wants to rename the variables, then I would guess that a test with Patchy would be sufficient.
Labels: -Patch-countdown Patch-push
Originally posted by: dak@gnu.org
Pushed as [rc37c6f39d18274ccac28ed42559681ea271cc496] to staging. It fixes an existing bug while keeping the documented interfaces, and I don't see that we have a good redesign of the structure of output definitions and books just lurking around the corner where this would be a nicer fit.
As long as \midi is not tied into books at all, the proposed redesigns so far don't really address the underlying design discrepancies.
Labels: -Patch-push Fixed_2_15_31
Status: Fixed_2_15_31
Originally posted by: dak@gnu.org
(No comment was entered for this change.)
Status: Fixed
Originally posted by: colingh...@gmail.com
Verified fixed in release 2.15.31 using WinXP SP3 x86.
Status: Verified
Originally posted by: PhilEHol...@googlemail.com
Reverted to fixed, since this needs reverifying by checking the source as pulled from git, not just the commitish.
Status: Fixed
Originally posted by: Elu...@gmail.com
verified with 2.15.33
Status: Verified