Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(520)

Issue 325470043: Allow a markup to replace the default LyricHyphen

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 6 months ago by knupero
Modified:
6 years, 5 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Allow a markup to replace the default LyricHyphen Solves issue #1255. Signed-off-by: Knut Petersen <knut_petersen@t-online.de>

Patch Set 1 #

Total comments: 2

Patch Set 2 : Version 2 of issue #1255 patch #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -24 lines) Patch
M lily/lyric-hyphen.cc View 1 5 chunks +48 lines, -24 lines 4 comments Download
M scm/define-grobs.scm View 1 1 chunk +1 line, -0 lines 3 comments Download

Messages

Total messages: 9
dak
https://codereview.appspot.com/325470043/diff/1/lily/lyric-hyphen.cc File lily/lyric-hyphen.cc (right): https://codereview.appspot.com/325470043/diff/1/lily/lyric-hyphen.cc#newcode55 lily/lyric-hyphen.cc:55: bool use_markup = to_boolean (me->get_property ("use-markup")); That property seems ...
6 years, 6 months ago (2017-09-14 09:34:04 UTC) #1
knupero
Version 2 of issue #1255 patch
6 years, 6 months ago (2017-09-14 10:48:10 UTC) #2
knupero
https://codereview.appspot.com/325470043/diff/1/lily/lyric-hyphen.cc File lily/lyric-hyphen.cc (right): https://codereview.appspot.com/325470043/diff/1/lily/lyric-hyphen.cc#newcode55 lily/lyric-hyphen.cc:55: bool use_markup = to_boolean (me->get_property ("use-markup")); On 2017/09/14 09:34:03, ...
6 years, 6 months ago (2017-09-14 10:57:30 UTC) #3
dak
Rats, forgot to publish my code comments. Indepent of those: I don't actually see this ...
6 years, 6 months ago (2017-09-16 19:53:57 UTC) #4
knupero
On 2017/09/16 19:53:57, dak wrote: > Indepent of those: I don't actually see this addressing ...
6 years, 6 months ago (2017-09-17 10:02:41 UTC) #5
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 + ...
6 years, 6 months ago (2017-09-17 10:16:01 UTC) #6
dak
On 2017/09/17 10:02:41, knupero wrote: > On 2017/09/16 19:53:57, dak wrote: > > > Indepent ...
6 years, 6 months ago (2017-09-17 12:36:26 UTC) #7
dak
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#newcode126 lily/lyric-hyphen.cc:126: Stencil dash_mol (Lookup::round_filled_box (b, 0.8 * lt)); On 2017/09/17 ...
6 years, 6 months ago (2017-09-17 12:42:58 UTC) #8
knupero
6 years, 5 months ago (2017-10-04 12:32:05 UTC) #9
> Issue 1255 is: "#1255 Extract hyphen dimensions and/or hyphen glyph from the
> font "
> 
> Your patch does not extract hyphen dimensions or the hyphen glyph from the
font.
>  It might get used for using the hyphen glyph (provided that you know its
> font-dependent character code) as a hyphen, so it might contribute to deciding
> to retire issue #1255 "Wontfix".
> 

I opened a new issue 5210 and uploaded extended code for review.

So this issue here is dead.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b