Re: Allow a markup to replace the default LyricHyphen (issue 325470043 by address@hidden)

[knupero]

https://codereview.appspot.com/325470043/diff/20001/lily/lyric-hyphen.cc
File lily/lyric-hyphen.cc (right):

https://codereview.appspot.com/325470043/diff/20001/lily/lyric-hyphen.cc#newcode125
lily/lyric-hyphen.cc:125: Box b (Interval (0, dash_length), Interval (h,
h + th));
On 2017/09/16 19:53:56, dak wrote:
> It seems wasteful to put these in the loop, particularly since
dash_mol is then
> additionally copied before shifting.  Maybe pull if
(Text_interface::is_markup
> ...) outside of the loop and have a separate loop for each branch?
There is no
> shared loop code outside of if/else in the current implementation
anyway.

Well, you are right.

https://codereview.appspot.com/325470043/diff/20001/lily/lyric-hyphen.cc#newcode126
lily/lyric-hyphen.cc:126: Stencil dash_mol (Lookup::round_filled_box (b,
0.8 * lt));
Shouldn't the hardcoded 0.8 be replaced by a property?

https://codereview.appspot.com/325470043/diff/20001/scm/define-grobs.scm
File scm/define-grobs.scm (right):

https://codereview.appspot.com/325470043/diff/20001/scm/define-grobs.scm#newcode1403
scm/define-grobs.scm:1403: (text . '())
On 2017/09/16 19:53:56, dak wrote:
> That's the default when unset.  For properties that may optionally be
set, in
> particular when this default formally does not match the predicate, I
think we
> tend not to explicitly specify it.

Well, omitting it here means that there is no indication in the
internals reference that LyricHyphen uses that property.

https://codereview.appspot.com/325470043/