Re: note-name-engraver: add user-defined note names support (issue 221710044 by address@hidden)

[v . villenave]

On 2019/02/11 22:02:11, dak wrote:
> There is alist->hash-table in scm/lily-library.scm

Which is a similar to ice-9’s alist->hashq-table…

> For printing note names, there is note-name->lily-string exported from
> scm/define-music-display-methods.scm which, well, actually does use
> rassoc.

Yes, I believe it was originally added by Nicolas Sceaux.  I was tempted
to turn it into a faster function, but we can certainly make do with
his.  That question is actually entirely separate from the NoteNames
feature.

> Maybe we should just write a hashed-rassoc that uses a weak-key
> table for associating alists with hash-tables?  One should not be
using
> it on destructively modified alists, of course.

Sure.  Thanks for the draft, by the way.

> Something like

> (define rassoc-hashhash (make-weak-key-hashtable))
> (define (hashed-rassoc key alist default)
>    (let ((hasher (hashq-create-handle! rassoc-hashhash alist #f)))
>      (if (not (cdr hasher))
>          (set-cdr! hasher (alist->hash-table alist)))
>      (hashq-ref (cdr hasher) key default)))

> It's not like there aren't a few other uses for it, not too few in the
> display-lily machinery.  One would need to think about how to support
> the hashq/hashv/hash triple.

Indeed.  I’ll give it a proper look (if pure Scheme implementation is
the way to go, then I’m a lot less out of my depth).

> There are several loose ends worth getting tied up.  Now the question
is
> what parts I should be doing for you not to lose interest.

That’s very kind of you, but hardly the point.  This patch set (as well
as the not-null? patch) has allowed me to learn quite a bit more about
the way primitive Scheme procedures and assertions are implemented,
anyway.

> Doing a single call of
> make_markup_concat (scm_list_3 (markup1, markup2, markup3));
> does not seem too bad and the export/import is a one-time cost for all
> prospective uses.  I am surprised we don't have it already, actually.

OK, now it works with markups… but as a result, what it returns is a
whole stencil instead of a 'text property that we can play with further
down the line. *And* the code could be a lot more elegant, since it
forced me to make numerous nested lists (some of which may not be
strictly necessary).

My plan is to get this particular feature merged using Nicolas’ rassoc
function, and then we can discuss how to make a rassoc function more
streamlined/scalable etc.

V.

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