openlilylib / snippets

A place to store useful pieces of LilyPond code - custom functions, engravers, hacks, templates, examples etc.
Other
120 stars 38 forks source link

Allow assigning individual items to a GridLY cell #104

Closed uliska closed 9 years ago

uliska commented 9 years ago

I think it would be a nice enhancement to be able to assign only individual items to an existing grid cell or segment template. This may make sense in contexts where the bulk of things is done automatically.

I see two alternatives:

And (it could be seen as a separate wish but I think they can be considered (and implemented) together) while we're at it: wouldn't it be a good idea to change the signature of \gridPutMusic and "pull" the music argument into the \with {} block?

I'd make the function use

(parser location opts-or-music)
  (context-mod-or-music?)

with context-mod-or-music? being a new predicate. This would allow for the following two invocation options:

\gridPutMusic "soprano" 1 \with {
  lyrics = \lyricmode { This is the text }
  music = \relative c' { c4 d e f }
}

\gridPutMusic "soprano" 1 { c d e f }

I find this clearer than having the music as a separate argument after the \with block. This is particularly true when the \with block has some extent (as can be seen in this file). Here it is not really obvious to the reader (and maybe also to the one who has to fill out the template) that the music block is actually an argument to \griPutMusic.

Cecca commented 9 years ago

I think it would be a nice enhancement to be able to assign only individual items to an existing grid cell or segment template.

For grid cells I don't see a use case for assigning items individually, I think that it's better to define everything that is related to a cell in one go, in a single place.

As for segment templates, can you make an example when assigning individual items to a segment template is useful? In any case, I'd prefer to go for the \addToSegmentTemplate command, in order to be explicit.


As for the other issue, I'm not sure. First of all, it looks to me more like a documentation issue, rather than an API issue. Anyway, if we decide to pull the music argument inside the with block I'd prefer to keep this new interface as the only one available. So a cell with only music should be defined as

\gridPutMusic "soprano" 1 \with {
  music = \relative c' { c4 d e f }
}

This is something I thought when first defining the API for gridly. I decided to go for the separate argument for the music because the music is the only mandatory thing that has to be included in a grid. So to me it made sense making it explicit in the API. The drawback is that the music argument is not named, but this can be addressed in the documentation,

uliska commented 9 years ago

In "Das trunkne Lied" I have a function that iterates over a number of music variables to set the segment templates' music:

vocalTemplates =
#(define-void-function (parser location)()
   ;; Look up all music variables (loaded through empty-segments.ily)
   ;; and set the segment templates with MultiMeasureRests of the same length
   (for-each
    (lambda (i)
      (let ((music
             ;; Look up the music variable whose name is
             ;; the roman numeral of the segment index
             (ly:parser-lookup parser
               (string->symbol (format "~@r" i)))))
        #{ \gridSetSegmentTemplate #i
           \with { lyrics = \lyricmode { } }
           #(mmrest-of-length music)
        #}))
    (iota 90 1)))

\vocalTemplates

defined in https://git.ursliska.de/beautifulscores/das-trunkne-lied/blob/master/library/ly/makescore/gridly.ily.

As this is about choir music it may become necessary (and there surely is music where this will become possible) to add lyrics to the segment templates, but this isn't easily possible when setting the segment templates automatically like above. The same might be true for transpositions or items in the opening (although I don't have an example for that).

But I agree that this should only be done for segment templates and not for actual cells.


Regarding the second issue: In theory you're right that it is a documentation issue. And also the consideration about the mandatory argument is correct. Nevertheless, if you take the following real-world example (I had linked to the file but I quote it for explicitness now) it simply gets non-obvious that the music is actually an argument to \gridPutMusic (and this cell's music isn't even very long). Having the music as an item in the \with {} block would make it much more idiomatic.

However, I'm ok with having only one interface to it (although this will require me to update numerous existing files - but that should be possible to do automatically).

\gridPutMusic "voc-soprano-I" 6
\with {
  lyrics = \lyricmode {
    \getCommonLyrics "06-1"
    wan -- deln.
  }
  barNumber = 46
  transposeKey = c
  opening = {
    \time 3/4
    \key es \minor
    % add optional opening music here
    \mark 5
  }
  closing = {
    \mark 6
    % add optional closing music here
  }
}
% Please check the reference pitch of \relative
\relative a' {
  \barNumberCheck 46
  R2.

  %47
  r8 a8 \p bes c es d

  %48
  f8 es f ges as4 ~ ( |

  %\origBreak %7
  %49
  as2. ~ |

  %50
  as4 f es )

  %51
  d4 r r |

  %52
  R2.
}
Cecca commented 9 years ago

@uliska, after having added another optional argument to the with block of GridLY function in #115, I agree with you that the interface of GridLY functions is growing more and more confusing. Hence, I implemented the changes you suggested in this discussion. Now gridPutMusic and gridSetSegmentTemplate both accept either a music block or a contest modifier that must contain the music among the other arguments.

Unfortunately, due to the way LilyPond handles optional arguments, I wasn't able to preserve backwards compatibility (like you, I have lots of files relying on the old interface), hence I introduced a couple of aliases with the same calling convention of the old version of the functions. I wrote a short migration guide to help in the process.

uliska commented 9 years ago

I think one aspect is handled and the other is "invalid"