Re: Don't wrap EventChord around rhythmic events by default. (issue 5440084)

[dak]

http://codereview.appspot.com/5440084/diff/14005/lily/music-scheme.cc
File lily/music-scheme.cc (right):

http://codereview.appspot.com/5440084/diff/14005/lily/music-scheme.cc#newcode79
lily/music-scheme.cc:79: "Is @var{obj} a proper (non-rhythmic) event
object?")
On 2012/01/24 22:47:32, Neil Puttock wrote:
> I'd be much happier if there were a separate ly:post-event? predicate.
 If I see
> ly:event? I'd expect it to return true for all events.

I did not change DOC string (which has stopped being even closely
accurate some time ago) or name at this stage.  The music display code
contains a separate post-event? function that has been folded into this.

One should eventually narrow down on a single definition and change docs
(and convert-ly) rules accordingly.  The post-event? stuff has already
been committed separately.

I think that ly:post-event? would likely make most sense, even though it
is neither of the preexisting definitions.

http://codereview.appspot.com/5440084/diff/14005/lily/music.cc
File lily/music.cc (right):

http://codereview.appspot.com/5440084/diff/14005/lily/music.cc#newcode164
lily/music.cc:164: (void) music_list_to_relative (get_property
("articulations"), last, true);
On 2012/01/24 22:47:32, Neil Puttock wrote:
> Since this is only used in TrillSpanEvent, wouldn't it be better to
have a
> callback specific to rhythmic music which does this instead of
checking it every
> time?

Not sure about that.

Articulations tend not to be all that many.

> Ideally, we'd have a separate event for the trill pitch, then we
wouldn't need
> to do this at all if it were kept out of 'articulations (and it would
have the
> added benefit of correct origin info).

"Separate event" is not necessarily fun.  In the first iteration, I
don't want to tackle all too many complex changes and intricacies but
rather keep things working as simple as possible.  That does not
preclude patches to better tune the behavior.

In any way, if a trill afternote is a post-event, it ends up as an
articulation on single notes.  Simple rules, simple effects.  Why it is
a post-event, I don't know, but it is not the topic of this patch to
change that.  And I don't know whether it is really the only pitched
articulation around.

http://codereview.appspot.com/5440084/diff/14005/scm/define-music-display-methods.scm
File scm/define-music-display-methods.scm (right):

http://codereview.appspot.com/5440084/diff/14005/scm/define-music-display-methods.scm#newcode141
scm/define-music-display-methods.scm:141: (define (post-event? m)
On 2012/01/24 22:47:32, Neil Puttock wrote:
> This just duplicates the code for the exported predicate.

Correct.  You'd rather have it refer to there instead?  I've not yet
decided which of post-event? and ly:event? has to go, and such a
decision would need a convert-ly rule.  So I prefer postponing it.

parser.yy also calls is_mus_type ("post-event") directly. Another
duplication.  But the calling indirection does not really seem worth the
gain.

http://codereview.appspot.com/5440084/