[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/
- Re: note-name-engraver: add user-defined note names support (issue 221710044 by address@hidden),
dak <=