[Top][All Lists]

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

Re: Charles Winston's chord-semantics GSOC work (issue 568650043 by addr

From: Carl . D . Sorensen
Subject: Re: Charles Winston's chord-semantics GSOC work (issue 568650043 by address@hidden)
Date: Sat, 06 Apr 2019 15:06:21 -0700

Thanks for the feedback!  New patch set uploaded.
File Documentation/notation/chords.itely (right):
Documentation/notation/chords.itely:472: c:m7^1 ees
On 2019/04/02 21:20:49, Valentin Villenave wrote:
I’d put the simple chord in front of the more unexpected use case.
Also, why not use c:maj7^1 and e:m chords?

File input/regression/ (right):
input/regression/ \version "2.16.0"
On 2019/04/02 21:20:49, Valentin Villenave wrote:
Why not update the regtest version number? OTOH, it doesn’t actually
a new syntax.

File input/regression/ (right):
input/regression/ }
On 2019/04/02 21:20:49, Valentin Villenave wrote:
I feel like these are way too many regtests.  Sure, having a nicely
ordered list
of all features looks nice, but 1/ we might be adding quite a few
extra seconds
to `make check’ here 2/ regtests are not the place where to be listing
documenting features and 3/ all of these could as well be included in
a single
regtest, duly commented and explained: if anything ever breaks in the
we’ll catch it just as well.

Upon review, I agree with you.  This is really a set of regtests for the
chord-semantics chord namer.  I've combined them into a single regtest.
File lily/ (right):
lily/ SCM name_proc = get_property
On 2019/04/02 21:20:49, Valentin Villenave wrote:
Ugh. Is there really no way of merging this with chordNameFunction
with  additionaloptargs)? I understand this adds many additional (and
useful) features, but this looks like potential for
subroutines down the line.

Fortunately, both approaches use chordRootNamer and therefore will be
able to
take advantage from the localized note names that are now available.
But still,
I wish we could factorize things even further.

With this patch, there are two fundamental modes of getting the
semantics necessary to define a chord name.  The newly-added one is to
use the semantics entered by the user (chordSemanticsNameFunction); the
previous one was to  parse the pitches in the chord and try to infer the
semantics.  (There is also the pattern-matching provided by the
exceptions that allows overriding the automatic calculation of chord
names, replacing it with a lookup function.)

Since the difference between the two is how we get the semantics, I
could see that we replace the two fundamental chord name functions with
a single function having an optional argument which is a semantics
entry.  If we want to use the semantics entry present in the chord, we
don't pass the optional argument.  If we want to use the pitch parser
chord namer, we could create a new function whose job is to parse a set
of pitches into a semantics entry.  The resulting semantics entry could
then be passed to the semantics-based namer.

Or alternatively, we could always call the semantic namer, and have the
semantic namer call the parse-semantics function if there is no
semantics entry in the chord.  I believe this could be a way to achieve
your goal.

However, this will require more refactoring.  I don't believe we should
hold off on this patch until we can get that part of it done better.
This patch has lanquished long enough.  IMO we should just push it as-is
and get it in the code base.
File scm/chord-ignatzek-names.scm (right):
scm/chord-ignatzek-names.scm:342: (define (make-root-markup root
On 2019/04/02 21:20:49, Valentin Villenave wrote:
I’m not sure "make-*-markup is the most unambiguous name for that

Good catch.

I've changed make-*-markup to make-*-display for all of the functions
that are local to this file.  Of course, the lillypond-defined
make-*-markup functions are not changed.

reply via email to

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