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

[dak]

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#newcode36
lily/script-interface.cc:36: Script_interface::get_glyph_or_stencil
(Grob *me, Direction d)
On 2019/02/22 20:41:08, Valentin Villenave wrote:
> On 2019/02/22 16:19:53, dak wrote:
> > There is no point in renaming this into get_glyph_or_stencil if it
cannot
> return
> > anything but a stencil.

> The point was to avoid any risk of confusion with Grob::get_stencil
(). If you
> find it unnecessary, I’ll remove it.

Where would the risk for confusion lie?

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)))
On 2019/02/22 20:41:08, Valentin Villenave wrote:
> On 2019/02/22 16:19:53, dak wrote:
> > Should be something like
> > if (Stencil *stil = unsmob<Stencil> (s))
> >   return *stil;

> Done.

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

> Well, that’s a problem because most stencil expressions do
syntactically satisfy
> the scm_is_pair test above. So all I could come up with was to let the
test
> unfold in case it’s a valid glyph lookup, and only then test for a
possible
> Stencil smob.

You are confusing stencils with stencil expressions.  Stencils satisfy
ly:stencil? and you can extract their stencil expression (which usually
is a pair) with ly:stencil-expr .

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
On 2019/02/22 20:41:08, Valentin Villenave wrote:
> On 2019/02/22 16:19:53, dak wrote:
> > 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.

> Are you referring to the Script.stencil property?  That wouldn’t be of
much use
> since this property affects all Script objects whereas script-stencil
> definitions allow to define many distinct articulations.  So your
callback would
> have to refer to an alist to look up appropriate glyphs/stencils
anyway.

> > Then you can just override stencil if you
> > want to specify a specific stencil.

> That’s already what we’re doing right now, no change needed
whatsoever.  But
> it’s a rather clunky hack, and doesn’t integrate well with the alist
> definitions.  (But I may be the one missing something here…)

I don't understand where the difference in what you do with
script-stencil as a property and what you would do with stencil as a
property would lie.

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