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

Issue 313240043: Automatic LyricExtenders

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by akobel
Modified:
1 month ago
Reviewers:
dak, knut_petersen, Trevor Daniels, Dan Eble, a-kobel, david.nalesnik, pkx166h
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Automatic LyricExtenders Fixes https://sourceforge.net/p/testlilyissues/issues/4509 As written and communicated by Knut Peterson. For reference, see the threads http://lists.gnu.org/archive/html/lilypond-devel/2016-12/msg00223.html http://lists.gnu.org/archive/html/lilypond-devel/2016-12/msg00121.html http://lists.gnu.org/archive/html/lilypond-user/2016-12/msg00523.html http://lists.gnu.org/archive/html/lilypond-user/2016-12/msg00303.html (sorry, this discussion grew quite long in the meantime).

Patch Set 1 #

Patch Set 2 : Lyric autoextenders: Silence some warnings #

Total comments: 9

Patch Set 3 : Tell parser to ignore "__" token #

Total comments: 6

Patch Set 4 : Version 2017-01-01 #

Patch Set 5 : Automatic lyric extenders - patchset 2017-01-15 #

Patch Set 6 : remove __ from regtestsx #

Patch Set 7 : hide deprecated token for ExtenderEvents in \displayLilyMusic #

Patch Set 8 : purge superfluous __ from Lilypond code #

Total comments: 1

Patch Set 9 : let __ return SCM_UNSPECIFIED; minor change in deprecation warning #

Total comments: 1

Patch Set 10 : Version 2017-02-04 by Knut #

Total comments: 12
Unified diffs Side-by-side diffs Delta from patch set Stats (+291 lines, -200 lines) Patch
M Documentation/learning/common-notation.itely View 6 chunks +10 lines, -12 lines 0 comments Download
M Documentation/notation/vocal.itely View 1 2 3 23 chunks +118 lines, -148 lines 0 comments Download
M lily/extender-engraver.cc View 1 2 3 4 5 6 7 8 9 7 chunks +35 lines, -12 lines 4 comments Download
M lily/lyric-extender.cc View 1 2 3 4 5 6 7 8 9 2 chunks +50 lines, -23 lines 3 comments Download
M lily/self-alignment-interface.cc View 1 2 3 3 chunks +6 lines, -3 lines 3 comments Download
M ly/music-functions-init.ly View 1 2 3 4 5 6 7 8 9 5 chunks +53 lines, -1 line 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 3 chunks +14 lines, -0 lines 2 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 30
akobel
For the sake of completeness: deserves an entry in changes.tely (TBD when the syntax discussion ...
3 months ago (2016-12-23 21:34:49 UTC) #1
akobel
Lyric autoextenders: Silence some warnings
3 months ago (2016-12-24 00:11:09 UTC) #2
Trevor Daniels
LGTM, with the trivial suggestions indicated below to the NR, but I'm not competent to ...
2 months, 4 weeks ago (2016-12-25 10:52:13 UTC) #3
akobel
Tell parser to ignore "__" token
2 months, 4 weeks ago (2016-12-25 14:03:09 UTC) #4
akobel
https://codereview.appspot.com/313240043/diff/40001/Documentation/notation/vocal.itely File Documentation/notation/vocal.itely (right): https://codereview.appspot.com/313240043/diff/40001/Documentation/notation/vocal.itely#newcode842 Documentation/notation/vocal.itely:842: @samp{ \markup\null } in these cases. I'm only 90% ...
2 months, 4 weeks ago (2016-12-25 14:27:25 UTC) #5
Knut_Petersen_t-online.de
Hi everybody! > I'm only 90% happy about "" and \markup\null... > For dynamic spanners ...
2 months, 4 weeks ago (2016-12-25 15:43:05 UTC) #6
david.nalesnik
https://codereview.appspot.com/313240043/diff/40001/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): https://codereview.appspot.com/313240043/diff/40001/lily/self-alignment-interface.cc#newcode61 lily/self-alignment-interface.cc:61: return scm_from_double (- ext.linear_combination (scm_to_double (align))); Grobs are recognized ...
2 months, 4 weeks ago (2016-12-25 17:25:27 UTC) #7
Dan Eble
On 2016/12/25 14:03:09, akobel wrote: > Tell parser to ignore "__" token I apologize if ...
2 months, 4 weeks ago (2016-12-25 19:13:32 UTC) #8
akobel
On 2016/12/25 15:43:05, Knut_Petersen_t-online.de wrote: > Hi everybody! > > > I'm only 90% happy ...
2 months, 4 weeks ago (2016-12-25 21:53:55 UTC) #9
pkx166h
On 2016/12/25 21:53:55, akobel wrote: > On 2016/12/25 15:43:05, Knut_Petersen_t-online.de wrote: > > Hi everybody! ...
2 months, 3 weeks ago (2016-12-26 19:14:00 UTC) #10
akobel
On 2016/12/26 19:14:00, pkx166h wrote: > On 2016/12/25 21:53:55, akobel wrote: > > Bottom line: ...
2 months, 3 weeks ago (2016-12-27 02:01:18 UTC) #11
Knut_Petersen_t-online.de
Am 27.12.2016 um 03:01 schrieb perpeduumimmobile@gmail.com: > On 2016/12/26 19:14:00, pkx166h wrote: >> On 2016/12/25 ...
2 months, 3 weeks ago (2016-12-27 20:55:21 UTC) #12
Knut_Petersen_t-online.de
Hi everybody! Attached find a new version of the autoextender patch set. @Alexander: I cannot ...
2 months, 3 weeks ago (2017-01-01 15:39:39 UTC) #13
akobel
Version 2017-01-01
2 months, 2 weeks ago (2017-01-02 09:45:45 UTC) #14
akobel
Version 2017-01-01
2 months, 2 weeks ago (2017-01-02 10:10:15 UTC) #15
a-kobel_a-kobel.de
A happy new one, everybody! On 2017-01-01 16:39, Knut Petersen wrote: > Hi everybody! > ...
2 months, 2 weeks ago (2017-01-02 10:13:39 UTC) #16
akobel
Automatic lyric extenders - patchset 2017-01-15
2 months, 1 week ago (2017-01-15 22:28:25 UTC) #17
akobel
remove __ from regtestsx
2 months, 1 week ago (2017-01-15 22:52:44 UTC) #18
akobel
hide deprecated token for ExtenderEvents in \displayLilyMusic
2 months ago (2017-01-17 17:55:52 UTC) #19
akobel
purge superfluous __ from Lilypond code
2 months ago (2017-01-17 23:34:40 UTC) #20
dak
https://codereview.appspot.com/313240043/diff/160001/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/313240043/diff/160001/lily/parser.yy#newcode3107 lily/parser.yy:3107: $$ = $1; That looks like strange fallback behavior. ...
2 months ago (2017-01-18 08:28:16 UTC) #21
Knut_Petersen_t-online.de
Am 18.01.2017 um 09:28 schrieb dak@gnu.org: > > https://codereview.appspot.com/313240043/diff/160001/lily/parser.yy > File lily/parser.yy (right): > > ...
2 months ago (2017-01-18 16:06:39 UTC) #22
akobel
let __ return SCM_UNSPECIFIED; minor change in deprecation warning
1 month, 4 weeks ago (2017-01-23 21:21:22 UTC) #23
dak
https://codereview.appspot.com/313240043/diff/40001/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): https://codereview.appspot.com/313240043/diff/40001/lily/self-alignment-interface.cc#newcode61 lily/self-alignment-interface.cc:61: return scm_from_double (- ext.linear_combination (scm_to_double (align))); On 2016/12/25 17:25:26, ...
1 month, 3 weeks ago (2017-01-30 15:11:22 UTC) #24
dak
https://codereview.appspot.com/313240043/diff/180001/lily/lyric-extender.cc File lily/lyric-extender.cc (right): https://codereview.appspot.com/313240043/diff/180001/lily/lyric-extender.cc#newcode62 lily/lyric-extender.cc:62: while (hs && robust_scm2double (heads[hs-1]->get_property This appears intended to ...
1 month, 3 weeks ago (2017-01-31 09:36:44 UTC) #25
akobel
Version 2017-02-04 by David
1 month, 2 weeks ago (2017-02-06 11:02:56 UTC) #26
dak
On 2017/02/06 11:02:56, akobel wrote: > Version 2017-02-04 by David Not "by David" but "by ...
1 month, 2 weeks ago (2017-02-06 18:38:37 UTC) #27
dak
On 2017/02/06 18:38:37, dak wrote: > On 2017/02/06 11:02:56, akobel wrote: > > Version 2017-02-04 ...
1 month, 2 weeks ago (2017-02-06 18:38:57 UTC) #28
dak
It's not really clear to me where I am supposed to go from here with ...
1 month, 2 weeks ago (2017-02-06 19:08:18 UTC) #29
dak
1 month ago (2017-02-16 13:01:37 UTC) #30
https://codereview.appspot.com/313240043/diff/200001/lily/lyric-extender.cc
File lily/lyric-extender.cc (right):

https://codereview.appspot.com/313240043/diff/200001/lily/lyric-extender.cc#n...
lily/lyric-extender.cc:63: while (hs && robust_scm2double
(heads[hs-1]->get_property
On 2017/02/06 19:08:18, dak wrote:
> Same as previously I see.

Which begs the question: what are the exact rules governing grace notes? 
Wouldn't it be sufficient to not even add any grace note to the "heads" array? 
Because at the time we add stuff there, the grace time of the time step is still
perfectly available.  Or are there situations where a grace note should be
present in "heads"?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted