Code review - Issue 4373046: Event listener to extract (some) music events.https://codereview.appspot.com/2011-06-05T22:29:35+00:00rietveld
Message from unknown
2011-04-07T09:42:41+00:00Graham Percival (old account)urn:md5:7643ec66cb52421f0ccaa1ae558d27be
Message from percival.music.ca@gmail.com
2011-04-07T09:44:17+00:00Graham Percival (old account)urn:md5:242aae9f0a363f950e2542719ea77623
First version of Vivi event listener. This is my first attempt at non-trivial scheme work; please be gentle. I fully expect to take half a dozen revisions before it's ok to push.
Message from v.villenave@gmail.com
2011-04-07T13:30:21+00:00Valentin Villenaveurn:md5:32f508406b518d520016ceb9d6589063
Greetings Graham,
this looks acceptable to me, although I'm certainly not the most qualified person in this regard.
Are you quite sure this really is generic enough, though? There are a few hardcoded things here, and more inconveniently this approach means you have to manually choose which events you're listening to. I do use a similar approach every now and then (without outputting it to a formatted file though) but for very specific purposes only.
If we were to had a "LilyPond SDK" distribution, if would fit there quite well, but I'm just not sure about the standard, user-oriented, distribution.
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly
File input/regression/event-listener-output.ly (right):
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode17
input/regression/event-listener-output.ly:17: st =
Is it really necessary to use a function for text spanners? (In which case you should seriously adding that to music-functions.ly) Oh, and you might as well want to make it postfix: http://git.savannah.gnu.org/cgit/opus-libre.git/tree/doc/snippets/postfix-text-spanners.ly#n12
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode40
input/regression/event-listener-output.ly:40: d16(\downbow cis b a) g4 \breathe e8\p( g) fis4
I'd \upbow here (a matter of taste? :)
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode45
input/regression/event-listener-output.ly:45: b4\p\<( d8 cis) d4(-. fis8-.^"II" e-.^"II")
Similarly: how about ais and eis instead of a and e?
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode56
input/regression/event-listener-output.ly:56: a16\mp e' a e' a,,32\f e' a e' r8 r4
Any slurs missing here?
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode69
input/regression/event-listener-output.ly:69: << \vlnone >>
Is this considered good/sugar-ish syntax?
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly
File ly/event-listener.ly (right):
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode40
ly/event-listener.ly:40: 2 )
What about .ily extensions? I'd use a regexp here, much simpler and more flexible.
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode87
ly/event-listener.ly:87: (+ 60 (ly:pitch-semitones pitch))
Do you want to hardcode that?
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode137
ly/event-listener.ly:137: (close p)))
This looks like a lot of duplicated code. Perhaps a generic function could be achievable:
(format-from-engraver engraver event 'text)
or even as a list:
(format-list-from-engraver engraver event
(list
'breathe
'articulation-type
'text
...
)))
in which case it could be more elegant to then cons the "formatter" to the "listeners" list.
Message from mtsolo@gmail.com
2011-04-07T13:42:54+00:00MikeSolurn:md5:5c18e19a6f4ab77dd475a89d48746a31
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly
File ly/event-listener.ly (right):
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode36
ly/event-listener.ly:36: (string-concatenate (list
Move close brackets up to previous argument
'instrumentName)))
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode40
ly/event-listener.ly:40: 2 )
This fails (I think) when people do batch file processing.
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode40
ly/event-listener.ly:40: 2 )
(+ (string-r-index (object->string (command-line)) #\sp)
Same formatting below
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode53
ly/event-listener.ly:53:
(/ (ly:moment-main-numerator moment)
(ly:moment-main-denominator moment))))
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode137
ly/event-listener.ly:137: (close p)))
+1
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode190
ly/event-listener.ly:190: (cdr (car (cdr (car (cdr (car (car
I'd remove this bit.
A while ago I had kicked around the idea of meta-data in LilyPond. I used it to compose a piece and it worked ok. For this sorta thing, it seems like implementing meta-data would be a good idea. Otherwise, you're making assumptions about the location of information that may be volatile in future versions of LilyPond.
Message from unknown
2011-04-08T03:02:46+00:00Graham Percival (old account)urn:md5:62f499d7cfcd70c77778c312fd9b64e0
Message from percival.music.ca@gmail.com
2011-04-08T03:09:46+00:00Graham Percival (old account)urn:md5:c81600333b4eff1cb5a329eb1d5932db
@Valentin's overall comment: we've had 2 or 3 people on -devel asking about this functionality in 2011, so I think it's definitely worth demonstrating the possibility -- and in many cases, the current file is enough for whatever they'd want to do. (maybe after adding one or two more things, such as ties)
The current file doesn't have all that many things hard-coded. Granted, the "staff instrument name -> filename" might seem a bit arbitrary, and the lack of supporting stuff like "lilypond foo.ly bar.ly" might not be ideal, but I think it's still a huge step forward for researchers who aren't intimately aware of our scheme stuff.
If there's strong opposition to having it in ly/, then I could live with it being in Documentation/snippets/new/. However, I think think the current file is general enough, and would be useful enough, that it's worth putting in it ly/ so that anybody can use it easily.
Massive refactoring; it should be much cleaner now. I kind-of suspect that I'll be asked to use optional arguments instead of print-one-two-three-four, but this is what I have so far.
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly
File input/regression/event-listener-output.ly (right):
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode17
input/regression/event-listener-output.ly:17: st =
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> Is it really necessary to use a function for text spanners? (In which case you
> should seriously adding that to music-functions.ly)
I've considered the text span format to be horrible for the past few years. I can't remember if I ever added an issue for it, though.
> Oh, and you might as well
> want to make it postfix:
> http://git.savannah.gnu.org/cgit/opus-libre.git/tree/doc/snippets/postfix-text-spanners.ly#n12
Hmm. That file needs a .0 for the version (or ideally, move that to 2.13.58 so that people can try to compile it), and you need an = after the texidoc.
When I tried it to use it in this regtest, though, I got some weird errors.
Could you propose adding your function to music-functions.ly ?
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode40
input/regression/event-listener-output.ly:40: d16(\downbow cis b a) g4 \breathe e8\p( g) fis4
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> I'd \upbow here (a matter of taste? :)
that would make the \breathe more awkward. It's easier for a human to control the bow at the frog, so being on an upbow for the g4 makes the \breathe and subsequent e8\p easier to play.
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode45
input/regression/event-listener-output.ly:45: b4\p\<( d8 cis) d4(-. fis8-.^"II" e-.^"II")
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> Similarly: how about ais and eis instead of a and e?
I wanted to really show that Vivi could play the same pitch as an open string, but with a finger on a different string.
Sure, it's obvious to a violinist that if you can play ais on the D string, then you can also play a. I'm happy with the loss of a bit of "harmonic strength" in order to clarify the technical ability.
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode56
input/regression/event-listener-output.ly:56: a16\mp e' a e' a,,32\f e' a e' r8 r4
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> Any slurs missing here?
in the pizz section? No, it's quite unusual to add slurs to pizz, other than the l.v. type of "slur" (which is more like a tie, anyway)
http://codereview.appspot.com/4373046/diff/1/input/regression/event-listener-output.ly#newcode69
input/regression/event-listener-output.ly:69: << \vlnone >>
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> Is this considered good/sugar-ish syntax?
Maybe? What did you have in mind?
I tend to always use << >> for my music expressions in a score so that if I ever add a \vlntwo or \cello, it doesn't go berserk. If I used a {} around the \vlnone, or if I put the \vlnone in directly, then adding a \vlntwo would blow up in interesting ways.
I suppose I could add some linebreaks and indentation?
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly
File ly/event-listener.ly (right):
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode36
ly/event-listener.ly:36: (string-concatenate (list
On 2011/04/07 13:42:54, MikeSol wrote:
> Move close brackets up to previous argument
> 'instrumentName)))
Done.
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode40
ly/event-listener.ly:40: 2 )
On 2011/04/07 13:42:54, MikeSol wrote:
> (+ (string-r-index (object->string (command-line)) #\sp)
>
> Same formatting below
Done.
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode40
ly/event-listener.ly:40: 2 )
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> What about .ily extensions? I'd use a regexp here, much simpler and more
> flexible.
You have an odd definition of "simpler", if you think that writing a regexp is simpler than cut&paste from LSR. :)
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode40
ly/event-listener.ly:40: 2 )
On 2011/04/07 13:42:54, MikeSol wrote:
> This fails (I think) when people do batch file processing.
hmm, probably. I don't have the parser, though (and I don't think I can get the parser from just the engraver?)
I'm open to suggestions for this... I couldn't remember any better way to do it, other than Neil's solution which relied on having the parser.
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode53
ly/event-listener.ly:53:
On 2011/04/07 13:42:54, MikeSol wrote:
> (/ (ly:moment-main-numerator moment)
> (ly:moment-main-denominator moment))))
done.
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode87
ly/event-listener.ly:87: (+ 60 (ly:pitch-semitones pitch))
On 2011/04/07 13:30:21, Valentin Villenave wrote:
> Do you want to hardcode that?
The 60 is for MIDI pitch value output. (well, I should definitely comment it!)
http://codereview.appspot.com/4373046/diff/1/ly/event-listener.ly#newcode190
ly/event-listener.ly:190: (cdr (car (cdr (car (cdr (car (car
On 2011/04/07 13:42:54, MikeSol wrote:
> I'd remove this bit.
oh, I know it's bad -- but how else can I get the text value of the left-hand bounds of a TextSpanner ? :(
> A while ago I had kicked around the idea of meta-data in LilyPond. I used it to
> compose a piece and it worked ok. For this sorta thing, it seems like
> implementing meta-data would be a good idea. Otherwise, you're making
> assumptions about the location of information that may be volatile in future
> versions of LilyPond.
I'd love to have a "user-event" (just like note-event and the like), which just contains a list which the user can put whatever he wants into. I could then put data into my user-event with the custom "\st string number" music function, expressive markings like "dolce" or "espr", etc.
I was going to look into that once the first step of the event listener was pushed.
Message from unknown
2011-04-11T08:44:05+00:00Graham Percival (old account)urn:md5:901579e465b17e89da1e92b5f48b47de
Message from percival.music.ca@gmail.com
2011-04-11T08:45:10+00:00Graham Percival (old account)urn:md5:fc7a72a671a7382218054a22692ee91e
I've generalized the (print-foo ...) functions as (print-line ...), and indented with lispindent.lisp.
Message from unknown
2011-04-15T08:13:44+00:00Graham Percival (old account)urn:md5:984d129a4ec5ee721be472fc89aeae45
Message from percival.music.ca@gmail.com
2011-04-15T08:17:11+00:00Graham Percival (old account)urn:md5:efaf2fe6d4cca103e1fdcf50000c842f
Thanks to Carl's help, I've fixed the text spanner formatting. I think it's ready to be pushed.
(I'll add it to the next patches countdown)
Message from hanwenn@gmail.com
2011-04-18T03:50:59+00:00hanwennurn:md5:dcae099ba55b47d8be25ce9831bfe7fc
http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly
File input/regression/event-listener-output.ly (right):
http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly#newcode5
input/regression/event-listener-output.ly:5: listeners. The .notes file which is output from this file is not
please make this print whatever is relevant with ly:progress, so we can catch differences
http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly#newcode7
input/regression/event-listener-output.ly:7: done during the Grand Organization Project."
can you put remarks like this in the bug tracker instead?
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly
File ly/event-listener.ly (right):
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode21
ly/event-listener.ly:21: % http://percival-music.ca/vivi.html
frankly, I don't understand why this should be part of lilypond? Can you expand on what other uses it may have?
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode24
ly/event-listener.ly:24: % from lilypond.
Document output format, perhaps with a small example.
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode38
ly/event-listener.ly:38: ( + ( string-rindex ( object->string ( command-line )) #\sp )
no spaces inside scheme parens, ie. "(a b)" iso. "( a b )"
Message from unknown
2011-04-18T07:49:14+00:00Graham Percival (old account)urn:md5:bf66a6a6ba7663d5c0130543f6d5665f
Message from percival.music.ca@gmail.com
2011-04-18T07:49:31+00:00Graham Percival (old account)urn:md5:be24fb46a9b571f5e5d833f7fd14d5a5
http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly
File input/regression/event-listener-output.ly (right):
http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly#newcode5
input/regression/event-listener-output.ly:5: listeners. The .notes file which is output from this file is not
On 2011/04/18 03:50:59, hanwenn wrote:
> please make this print whatever is relevant with ly:progress, so we can catch
> differences
done, I think. It looks a bit cludgy to me, but I definitely don't want to spam stuff to ly:progress all the time, or else it would be a pain to use it in real life.
http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly#newcode7
input/regression/event-listener-output.ly:7: done during the Grand Organization Project."
On 2011/04/18 03:50:59, hanwenn wrote:
> can you put remarks like this in the bug tracker instead?
no need with the new EVENT_LISTENER_CONSOLE_OUTPUT trick.
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly
File ly/event-listener.ly (right):
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode21
ly/event-listener.ly:21: % http://percival-music.ca/vivi.html
On 2011/04/18 03:50:59, hanwenn wrote:
> frankly, I don't understand why this should be part of lilypond? Can you expand
> on what other uses it may have?
Recent requests:
http://lists.gnu.org/archive/html/lilypond-devel/2011-01/msg00728.html
http://lists.gnu.org/archive/html/lilypond-user/2010-01/msg00657.html
solution inspired by:
http://lists.gnu.org/archive/html/lilypond-devel/2009-12/msg00884.html
Of course different researchers may want to examine different aspects of the output; the documentation will address that, and once they've modified event-listener.ly to their liking, they might send patches back to us. The important thing IMO is to have the basic framework there, so that researchers know that it's possible (and ideally how to get started).
I agree that there aren't many music researchers who want to use lilypond -- IIRC we have 2-4 emails about this each year -- but I think that's enough of a "market" to make this sensible. Besides, it's not like I'm modifying any scheme or C++ files, or even including something by default! Using \include "event-listener.ly" is a strictly optional thing.
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode24
ly/event-listener.ly:24: % from lilypond.
On 2011/04/18 03:50:59, hanwenn wrote:
> Document output format, perhaps with a small example.
I like small, focused patches. Documentation comes after the code is pushed.
http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode38
ly/event-listener.ly:38: ( + ( string-rindex ( object->string ( command-line )) #\sp )
On 2011/04/18 03:50:59, hanwenn wrote:
> no spaces inside scheme parens, ie. "(a b)" iso. "( a b )"
done.
Message from hanwenn@gmail.com
2011-04-18T14:09:52+00:00hanwennurn:md5:ad48137cd49ea4d91cb459cd23ff055c
On Mon, Apr 18, 2011 at 4:49 AM, <percival.music.ca@gmail.com> wrote:
>
> http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly
> File input/regression/event-listener-output.ly (right):
>
> http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly#newcode5
> input/regression/event-listener-output.ly:5: listeners. The .notes file
> which is output from this file is not
> On 2011/04/18 03:50:59, hanwenn wrote:
>>
>> please make this print whatever is relevant with ly:progress, so we
>
> can catch
>>
>> differences
>
> done, I think. It looks a bit cludgy to me, but I definitely don't want
> to spam stuff to ly:progress all the time, or else it would be a pain to
> use it in real life.
I cant see any changes. Did you upload?
> http://codereview.appspot.com/4373046/diff/12001/input/regression/event-listener-output.ly#newcode7
> input/regression/event-listener-output.ly:7: done during the Grand
> Organization Project."
> On 2011/04/18 03:50:59, hanwenn wrote:
>>
>> can you put remarks like this in the bug tracker instead?
>
> no need with the new EVENT_LISTENER_CONSOLE_OUTPUT trick.
>
> http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly
> File ly/event-listener.ly (right):
>
> http://codereview.appspot.com/4373046/diff/12001/ly/event-listener.ly#newcode21
> ly/event-listener.ly:21: % http://percival-music.ca/vivi.html
> On 2011/04/18 03:50:59, hanwenn wrote:
>>
>> frankly, I don't understand why this should be part of lilypond? Can
>
> you expand
>>
>> on what other uses it may have?
>
> Recent requests:
> http://lists.gnu.org/archive/html/lilypond-devel/2011-01/msg00728.html
> http://lists.gnu.org/archive/html/lilypond-user/2010-01/msg00657.html
>
> solution inspired by:
> http://lists.gnu.org/archive/html/lilypond-devel/2009-12/msg00884.html
>
> Of course different researchers may want to examine different aspects of
> the output; the documentation will address that, and once they've
> modified event-listener.ly to their liking, they might send patches back
> to us. The important thing IMO is to have the basic framework there, so
> that researchers know that it's possible (and ideally how to get
> started).
FWIW, it looks as if you're mostly interested in this at the
musical-semantic level. In that case, it would probably be cleaner to
hook into the event listener framework directly, without having an
engraver in between. The Scheme engraver mechanism is really for
creating formatted output rather than siphoning off data.
>> Document output format, perhaps with a small example.
>
> I like small, focused patches. Documentation comes after the code is
> pushed.
it's hard to do a sensible code review if I can't see what the code is
meant for.
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Message from percival.music.ca@gmail.com
2011-04-18T14:30:55+00:00Graham Percival (old account)urn:md5:aa2359f9dcdbf58410fe8434353fa5af
On 2011/04/18 14:09:52, hanwenn wrote:
> On Mon, Apr 18, 2011 at 4:49 AM, <mailto:percival.music.ca@gmail.com> wrote:
> >
> I cant see any changes. Did you upload?
I think so... when I click on "diff from patchset 4", I see the changes I made 6 hours ago.
> > Of course different researchers may want to examine different aspects of
> > the output; the documentation will address that, and once they've
> > modified event-listener.ly to their liking, they might send patches back
> > to us. &nbsp;The important thing IMO is to have the basic framework there, so
> > that researchers know that it's possible (and ideally how to get
> > started).
>
> FWIW, it looks as if you're mostly interested in this at the
> musical-semantic level.
Yes.
> In that case, it would probably be cleaner to
> hook into the event listener framework directly, without having an
> engraver in between. The Scheme engraver mechanism is really for
> creating formatted output rather than siphoning off data.
I have no clue how to do this? Is this simple to do with a .ly file? I definitely don't want to mess with C++ code or SCM files.
Quite apart from my ignorance of those areas, I want to let this serve as an example for other people interested in extracting info from lilypond. It's much easier to work on a .ly file which you just \include in your main file, rather than modifying scm files in the lilypond share dir, or changing C++ code and then recompiling.
> > I like small, focused patches. &nbsp;Documentation comes after the code is
> > pushed.
>
> it's hard to do a sensible code review if I can't see what the code is
> meant for.
Well, the easiest way to see what it's for is just to run the regtest right now; it will spam messages like
0.000 note 57 4 p-c 2 12
0.000 dynamic f
0.250 note 62 4 p-c 7 12
0.500 note 66 8 p-c 9 12
0.625 note 69 8 p-c 14 12
0.750 rest 0 4
to the console. That info can be easily read into python using open("foo.notes").readline(); line.split(), and then you can do whatever you want with it.
But ok, I'll write the docs now.
Message from percival.music.ca@gmail.com
2011-04-19T11:16:46+00:00Graham Percival (old account)urn:md5:c05c4e2f03592a4f2d74f7a2ce7e43b8
ok, doc patch uploaded to
http://codereview.appspot.com/4438054/
Message from hanwenn@gmail.com
2011-04-19T13:20:17+00:00hanwennurn:md5:50dd42c46933e630fce17ae7b9730e55
Right, but that is a doc patch. I am asking for comments in the code.
On Tue, Apr 19, 2011 at 8:16 AM, <percival.music.ca@gmail.com> wrote:
> ok, doc patch uploaded to
> http://codereview.appspot.com/4438054/
>
> http://codereview.appspot.com/4373046/
>
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Message from unknown
2011-04-19T15:35:17+00:00Graham Percival (old account)urn:md5:3255e06c07f2c4ca9ca597e8baa2d88d
Message from percival.music.ca@gmail.com
2011-04-19T15:37:19+00:00Graham Percival (old account)urn:md5:8f8d4e6f14ccf707aeb0198deca23dac
On 2011/04/19 13:20:17, hanwenn wrote:
> Right, but that is a doc patch. I am asking for comments in the code.
Oh, sorry.
Hmmm, I've never documented scheme before. Is this update along the right lines? Or did you want specifically ;; comments in places? I see that we use this type of "python docstring" in scm/*.scm files, and it still compiles, so presumably guile allows the same type of "function documentation" as python?
Message from hanwenn@gmail.com
2011-04-19T17:11:25+00:00hanwennurn:md5:d61b5a2e93b15bf536ecc6c3fa8d7717
LGTM.
On Tue, Apr 19, 2011 at 12:37 PM, <percival.music.ca@gmail.com> wrote:
> On 2011/04/19 13:20:17, hanwenn wrote:
>>
>> Right, but that is a doc patch. I am asking for comments in the code.
>
> Oh, sorry.
>
> Hmmm, I've never documented scheme before. Is this update along the
> right lines? Or did you want specifically ;; comments in places? I see
> that we use this type of "python docstring" in scm/*.scm files, and it
> still compiles, so presumably guile allows the same type of "function
> documentation" as python?
>
> http://codereview.appspot.com/4373046/
>
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Message from n.puttock@gmail.com
2011-04-22T18:16:55+00:00Neil Puttockurn:md5:0f5a9ff81e52b35d58ed9e035759fe4f
http://codereview.appspot.com/4373046/diff/12002/ly/event-listener.ly
File ly/event-listener.ly (right):
http://codereview.appspot.com/4373046/diff/12002/ly/event-listener.ly#newcode102
ly/event-listener.ly:102: #(define (print-line engraver values)
If you make `values' a `rest' argument you won't need to call print-line with a list, i.e.,
(print-line engraver . values)
then
(print-line engraver
"rest"
(ly:duration->string
etc.
Message from n.puttock@gmail.com
2011-04-22T18:25:19+00:00Neil Puttockurn:md5:d81ff1c7b711d1b6adf428871e42dfe6
On 18 April 2011 15:30, <percival.music.ca@gmail.com> wrote:
> On 2011/04/18 14:09:52, hanwenn wrote:
>> In that case, it would probably be cleaner to
>> hook into the event listener framework directly, without having an
>> engraver in between. The Scheme engraver mechanism is really for
>> creating formatted output rather than siphoning off data.
>
> I have no clue how to do this? Is this simple to do with a .ly file? I
> definitely don't want to mess with C++ code or SCM files.
You don't have to do that; there are exported functions which provide
all the hooks.
Here's an example which generates an ambitus markup:
http://lists.gnu.org/archive/html/lilypond-user/2010-02/msg00688.html
Cheers,
Neil
Message from unknown
2011-04-23T16:19:19+00:00Graham Percival (old account)urn:md5:5aed98b50896d077bb345f61b57d3330
Message from percival.music.ca@gmail.com
2011-04-23T16:34:00+00:00Graham Percival (old account)urn:md5:29cdd81a41c7eee439dea7bf3722088a
On 2011/04/22 18:16:55, Neil Puttock wrote:
> (print-line engraver . values)
Brilliant, thanks! Done.
Message from percival.music.ca@gmail.com
2011-04-23T16:47:14+00:00Graham Percival (old account)urn:md5:50802dc69abd70725ea1527a3f505d07
On 2011/04/22 18:25:19, Neil Puttock wrote:
> On 18 April 2011 15:30, <mailto:percival.music.ca@gmail.com> wrote:
> > On 2011/04/18 14:09:52, hanwenn wrote:
>
> >> In that case, it would probably be cleaner to
> >> hook into the event listener framework directly, without having an
> >> engraver in between. &nbsp;The Scheme engraver mechanism is really for
> >> creating formatted output rather than siphoning off data.
> >
> > I have no clue how to do this? &nbsp;Is this simple to do with a .ly file? &nbsp;I
> > definitely don't want to mess with C++ code or SCM files.
>
> You don't have to do that; there are exported functions which provide
> all the hooks.
>
> http://lists.gnu.org/archive/html/lilypond-user/2010-02/msg00688.html
Sorry, I'm still lost. Is the idea that I add stuff like this at the top level?
#(ly:add-listener
(ly:make-listener
format-note)
(ly:context-events-below
(ly:make-global-context $defaultlayout))
'note-event)
I definitely don't want to create a music function, since that would require manually editing lots of files -- it's much easier just to
cat "\include \"event-listener.ly\"" file-1234.ly >> tempfile.ly
rather than making some regex to add a music function call to the \score section of each file.
Unless there's some fantastic advantage that I'm missing (I doubt that there would be a big speed difference, and even if there was, speed isn't a primary concern), I think that modifying the Voice context is clearer from a "writing easily understandable code" perspective. Advanced users are already familiar with adding tweaks to a \layout \context, after all.
Message from hanwenn@gmail.com
2011-04-24T00:36:43+00:00hanwennurn:md5:02ffa40e388069f7f846d0b0b2fe6798
On Sat, Apr 23, 2011 at 1:47 PM, <percival.music.ca@gmail.com> wrote:
> Sorry, I'm still lost. Is the idea that I add stuff like this at the
> top level?
> #(ly:add-listener
> (ly:make-listener
> format-note)
> (ly:context-events-below
> (ly:make-global-context $defaultlayout))
> 'note-event)
>
> I definitely don't want to create a music function, since that would
> require manually editing lots of files -- it's much easier just to
> cat "\include \"event-listener.ly\"" file-1234.ly >> tempfile.ly
> rather than making some regex to add a music function call to the \score
> section of each file.
You can override how a toplevel music (and scores, for that matter) is
handled, so this is not a real argument.
Grep ly/declarations-init.ly for "toplevel-"
> Unless there's some fantastic advantage that I'm missing (I doubt that
> there would be a big speed difference, and even if there was, speed
You're passing all of the music through the typesetting process which
is just unnecessary.
--
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen
Message from percival.music.ca@gmail.com
2011-06-03T19:08:49+00:00Graham Percival (old account)urn:md5:d20b071070e4f697998b801c1a48beac
On 2011/04/24 00:36:43, hanwenn wrote:
> You're passing all of the music through the typesetting process which
> is just unnecessary.
Although most of the events I want to extract are actual music events, the format-textspan relies on information stored in an Engraver. Neil pointed out that I could examine Override events, but then I would need to store those values for later use.
I think it's more maintainable in the long run in the current format -- wait until the typesetting is done, then get any additional information I need from Engravers, using functions with no side effects (other than writing to the text file).
Message from percival.music.ca@gmail.com
2011-06-05T22:29:35+00:00Graham Percival (old account)urn:md5:c89cd54aba08ec8ad910af8df13c0da8
thanks all, this is now pushed.