[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Implementing Chord Semantics as a part of the EventChord structure,

From: Carl . D . Sorensen
Subject: Re: Implementing Chord Semantics as a part of the EventChord structure, (issue 321250043 by address@hidden)
Date: Tue, 18 Jul 2017 11:39:48 -0700

Looks generally good to me.  It's not yet complete, so I don't think
it's a candidate for pushing yet.  But I think you've got the right
stuff in and are moving forward well.

Good job!
File lily/ (right):
lily/ SCM name_proc = get_property
On 2017/07/06 21:57:23, Dan Eble wrote:
1. Should chordSemanticsNameFunction be included in the list at the
bottom of
this file? (I think so.)

chordSemanticsNameFunction should definitely be included as a property
that is read.
lily/ // Ugh, we created a grob, now we
better populate it.
On 2017/07/06 21:57:23, Dan Eble wrote:
If this was not expected, then as a user, I would like to see lilypond
emit a

I agree.
File scm/chord-entry.scm (right):
scm/chord-entry.scm:93: (interpret-additions (cons
(make-chord-entry-from-pitch (car mods))
To let you fit on an 80-column line, you may wish to move  (cons down to
the next line and indent it two spaces.  The other arguments to
interpret additions would then align with (cons
scm/chord-entry.scm:163: ;; TODO: this omits 3 in power chord. Change to
only do so if lead-mod is null.
Nice to note this; please make sure to add the extra clause in the if
scm/chord-entry.scm:206: (set! complete-chord (map (lambda (x)
(chord-pitch-transpose x root))
Maybe the name chord-pitch-transpose should be changed, since two things
are happening in that function:  1) The proper pitches are being created
(using ly:pitch-transpose) and
2) The given chord steps are being added.

Maybe make-chord-step or just chord-step

(set! complete-chord (map (lambda (x) (chord-step x root))

seems to be a bit clearer to me.  As it is, I wondered why you created
your own transpose function instead of using ly:pitch-transpose.  And
then I saw it was because the chord has both pitches and steps now.
scm/chord-entry.scm:309: ;; chord modifiers change the pitch list.
maybe change the comment to say chord modifiers change the pitch list
and the  chord steps?
scm/chord-entry.scm:349: (maj . , maj7-modifier)
I thought you were going to adjust this so maj was not maj7.  That way
c:5 could be power chord, and c:maj could be major triad.  c:maj7 would
be a major 7.

Is this plan gone now?
File scm/define-context-properties.scm (right):
scm/define-context-properties.scm:202: (chordSemanticsNameFunction
,procedure? "The function that converts
Should be "A" function, not "The" function, IMO.  See the parallel with

reply via email to

[Prev in Thread] Current Thread [Next in Thread]