Re: Set the sequence name in MIDI using title information from \header block (issue 256230045 by address@hidden)

[nine . fierce . ballads]

https://codereview.appspot.com/256230045/diff/1/lily/include/performance.hh
File lily/include/performance.hh (right):

https://codereview.appspot.com/256230045/diff/1/lily/include/performance.hh#newcode44
lily/include/performance.hh:44: void write_output (string filename,
const string &name) const;
It's not obvious what "name" is the name of.  A more descriptive name or
a comment would be helpful.

https://codereview.appspot.com/256230045/diff/1/lily/performance-scheme.cc
File lily/performance-scheme.cc (right):

https://codereview.appspot.com/256230045/diff/1/lily/performance-scheme.cc#newcode39
lily/performance-scheme.cc:39: Performance *p = unsmob<Performance>
(performance);
LY_ASSERT_SMOB returns a pointer, doesn't it?  So this unsmob is
redundant.

https://codereview.appspot.com/256230045/diff/1/lily/performance.cc
File lily/performance.cc (right):

https://codereview.appspot.com/256230045/diff/1/lily/performance.cc#newcode41
lily/performance.cc:41: header_ (SCM_EOL)
I don't work with Guile frequently enough to tell you whether or not you
need to do something to make sure that garbage collection works properly
with this SCM member.  David K. could probably tell you at a glance.

https://codereview.appspot.com/256230045/diff/1/lily/performance.cc#newcode81
lily/performance.cc:81: if (i == 0 && !s->audio_items_. empty())
no space after the dot; space before ()

https://codereview.appspot.com/256230045/diff/1/lily/performance.cc#newcode92
lily/performance.cc:92: text->text_string_ = name;
On 2015/08/05 03:58:25, lemzwerg wrote:
> This code allows overwriting `control track' only once.  I guess this
is
> intended, however, it looks limiting.  Wouldn't it be better to
identify the
> control track by another property, say Audio_text::CONTROL_TRACK_NAME,
instead
> of a comparison with a string that the user might modify?

In addition to that, recognizing the control track with (i == 0) seems
fragile.  (Please forgive me for criticizing without offering an
alternative.  I'm in a hurry.)

https://codereview.appspot.com/256230045/