lilypond-devel
[Top][All Lists]
Advanced

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

Re: note-name-engraver: add user-defined note names support (issue 22171


From: dak
Subject: Re: note-name-engraver: add user-defined note names support (issue 221710044 by address@hidden)
Date: Sun, 10 Feb 2019 04:23:40 -0800


https://codereview.appspot.com/221710044/diff/60001/lily/general-scheme.cc
File lily/general-scheme.cc (right):

https://codereview.appspot.com/221710044/diff/60001/lily/general-scheme.cc#newcode150
lily/general-scheme.cc:150: LY_DEFINE (ly_rassoc, "ly:rassoc",
I am not sure this is wanted (there are also other definitions) since
it's comparatively slow for large sets.  Can you think of a scheme using
a hashtable as cache?

https://codereview.appspot.com/221710044/diff/60001/lily/general-scheme.cc#newcode156
lily/general-scheme.cc:156: LY_ASSERT_TYPE (ly_cheap_is_list, alist, 2);
Using a predicate prone to make later code crash is not a good idea.
Just use ly_is_list instead.

https://codereview.appspot.com/221710044/diff/60001/lily/general-scheme.cc#newcode167
lily/general-scheme.cc:167: else if (ly_is_equal (val, key))
ly_is_equal is not eq? but equal?

https://codereview.appspot.com/221710044/diff/60001/lily/note-name-engraver.cc
File lily/note-name-engraver.cc (right):

https://codereview.appspot.com/221710044/diff/60001/lily/note-name-engraver.cc#newcode60
lily/note-name-engraver.cc:60: /* FIXME: what if we want to return a
markup? */
You can just call make-concat-markup for concatenating markup.  See
lily/lily-imports.cc and lily/include/lily-imports.hh to see how to haul
that function into C++ (assuming it isn't somewhere already).

https://codereview.appspot.com/221710044/



reply via email to

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