Allow scripts to be defined either as glyphs or stencils. (issue 348120043 by address@hidden)

[dak]

https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly
File input/regression/script-stencil.ly (right):

https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly#newcode31
input/regression/script-stencil.ly:31: #(append! default-script-alist
No.  Don't change any system data structures destructively or the
changes will persist across multiple files specified on the command
line.

https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly#newcode43
input/regression/script-stencil.ly:43: #(assoc-set!
Same here.

https://codereview.appspot.com/348120043/diff/1/lily/include/lily-imports.hh
File lily/include/lily-imports.hh (right):

https://codereview.appspot.com/348120043/diff/1/lily/include/lily-imports.hh#newcode86
lily/include/lily-imports.hh:86: extern Variable ly_stencil_p;
Unnecessary in C++.  Use unsmob<Stencil> instead.

https://codereview.appspot.com/348120043/diff/1/lily/lily-imports.cc
File lily/lily-imports.cc (right):

https://codereview.appspot.com/348120043/diff/1/lily/lily-imports.cc#newcode80
lily/lily-imports.cc:80: Variable ly_stencil_p ("ly:stencil?");
See above.

https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc
File lily/script-interface.cc (right):

https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc#newcode32
lily/script-interface.cc:32: #include "lily-imports.hh"
lily-imports.h should no longer be needed.

https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc#newcode36
lily/script-interface.cc:36: Script_interface::get_glyph_or_stencil
(Grob *me, Direction d)
There is no point in renaming this into get_glyph_or_stencil if it
cannot return anything but a stencil.

https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc#newcode59
lily/script-interface.cc:59: if (to_boolean (Lily::ly_stencil_p (s)))
That's rubbish.  Should be something like
if (Stencil *stil = unsmob<Stencil> (s))
  return *stil;

Actually, should likely be "else if" since there is no point in checking
again in case we ended with a programming_error above.

https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc#newcode144
lily/script-interface.cc:144: return get_glyph_or_stencil (me,
dir).smobbed_copy ();
See above.

https://codereview.appspot.com/348120043/diff/1/scm/c++.scm
File scm/c++.scm (right):

https://codereview.appspot.com/348120043/diff/1/scm/c++.scm#newcode47
scm/c++.scm:47: (define-public (pair-or-stencil? x)
Seems like "pair" is a stand-in for something more specific.  While the
predicate previously was just pair? likely for reasons of laziness, in
combination with stencil? it seems like something more specific would be
decidedly less awkward.

https://codereview.appspot.com/348120043/diff/1/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

https://codereview.appspot.com/348120043/diff/1/scm/define-grob-properties.scm#newcode1446
scm/define-grob-properties.scm:1446: (script-stencil ,pair-or-stencil?
"A pair @code{(@var{type} . @var{arg})} which
It seems to me like it would make more sense to leave script-stencil as
it is and instead put a callback into stencil that interprets it.  Then
whatever uses script-stencil now can instead get stencil (and thus read
and interpret script-stencil via the callback).  Then you can just
override stencil if you want to specify a specific stencil.  Am I
overlooking something here?

https://codereview.appspot.com/348120043/