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

Issue 7424049: Allows slurs to break at barlines.

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by MikeSol
Modified:
11 years ago
Reviewers:
Keith, janek, wl, mike7, Neil Puttock, x.scheuer, Ian Hulin (gmail), lemzwerg, dak, t.daniels, pkx166h
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Allows slurs to break at barlines.

Patch Set 1 #

Total comments: 1

Patch Set 2 : Uses breakSlurHere command #

Patch Set 3 : Allows either side of the broken slur to be junked #

Patch Set 4 : Small cleanups #

Patch Set 5 : From Neil's suggestions. #

Patch Set 6 : Breaks slurs at repeats by default. #

Total comments: 1

Patch Set 7 : Fixes bug in alternative count #

Patch Set 8 : Allows slurs to naturally stop and end at barlines. #

Patch Set 9 : Implements inheritence in slur engravers to avoid code dups #

Patch Set 10 : After large rebase #

Total comments: 1

Patch Set 11 : Rebases against master #

Total comments: 1

Patch Set 12 : Uses \broken command for manually starting and ending repeat slurs. #

Patch Set 13 : Adds copying of direction. #

Total comments: 1

Patch Set 14 : Correctly handles directions for broken slurs #

Patch Set 15 : Adds comments. #

Total comments: 1

Patch Set 16 : Two backslashes #

Total comments: 1

Patch Set 17 : Changes name, harmonizes slur direction #

Unified diffs Side-by-side diffs Delta from patch set Stats (+324 lines, -15 lines) Patch
A input/regression/repeat-slur.ly View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +162 lines, -0 lines 0 comments Download
M lily/include/slur-proto-engraver.hh View 1 2 3 4 5 6 7 8 9 10 3 chunks +3 lines, -0 lines 0 comments Download
M lily/phrasing-slur-engraver.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +9 lines, -0 lines 0 comments Download
M lily/slur.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +8 lines, -1 line 0 comments Download
M lily/slur-engraver.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +10 lines, -1 line 0 comments Download
M lily/slur-proto-engraver.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 6 chunks +77 lines, -12 lines 0 comments Download
M lily/spanner.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -0 lines 0 comments Download
M lily/volta-repeat-iterator.cc View 1 2 3 4 5 6 1 chunk +14 lines, -0 lines 0 comments Download
M ly/music-functions-init.ly View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +20 lines, -0 lines 0 comments Download
M ly/spanners-init.ly View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -0 lines 0 comments Download
M scm/define-context-properties.scm View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M scm/define-event-classes.scm View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -1 line 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +2 lines, -0 lines 0 comments Download
M scm/define-music-types.scm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 83
Neil Puttock
https://codereview.appspot.com/7424049/diff/1/input/regression/repeat-slur.ly File input/regression/repeat-slur.ly (right): https://codereview.appspot.com/7424049/diff/1/input/regression/repeat-slur.ly#newcode14 input/regression/repeat-slur.ly:14: \alternative { { a' ) b' \set breakSlurAtBarLine = ...
11 years, 2 months ago (2013-02-28 20:30:54 UTC) #1
MikeSol
Uses breakSlurHere command
11 years, 2 months ago (2013-02-28 20:32:04 UTC) #2
mike7
On 28 févr. 2013, at 21:30, n.puttock@gmail.com wrote: > > https://codereview.appspot.com/7424049/diff/1/input/regression/repeat-slur.ly > File input/regression/repeat-slur.ly (right): ...
11 years, 2 months ago (2013-02-28 20:37:28 UTC) #3
MikeSol
Allows either side of the broken slur to be junked
11 years, 2 months ago (2013-02-28 20:50:30 UTC) #4
MikeSol
Small cleanups
11 years, 2 months ago (2013-02-28 21:03:03 UTC) #5
Neil Puttock
On Feb 28, 2013 8:37 PM, "mike@mikesolomon.org" <mike@mikesolomon.org> wrote: > You're right, but I've opted ...
11 years, 2 months ago (2013-02-28 21:29:58 UTC) #6
MikeSol
From Neil's suggestions.
11 years, 2 months ago (2013-02-28 22:08:49 UTC) #7
mike7
On 28 févr. 2013, at 22:29, Neil Puttock <n.puttock@gmail.com> wrote: > > On Feb 28, ...
11 years, 2 months ago (2013-02-28 22:11:50 UTC) #8
MikeSol
Breaks slurs at repeats by default.
11 years, 2 months ago (2013-02-28 23:21:49 UTC) #9
MikeSol
Fixes bug in alternative count
11 years, 2 months ago (2013-02-28 23:33:46 UTC) #10
lemzwerg
Very nice, and thanks a lot! From visual inspection only, LGTM. Just curious: You apparently ...
11 years, 2 months ago (2013-02-28 23:42:43 UTC) #11
MikeSol
Allows slurs to naturally stop and end at barlines.
11 years, 2 months ago (2013-03-01 01:39:06 UTC) #12
lemzwerg
Much better, thanks!
11 years, 2 months ago (2013-03-01 06:13:36 UTC) #13
MikeSol
Implements inheritence in slur engravers to avoid code dups
11 years, 2 months ago (2013-03-01 10:56:57 UTC) #14
dak
On 2013/03/01 10:56:57, MikeSol wrote: > Implements inheritence in slur engravers to avoid code dups ...
11 years, 2 months ago (2013-03-01 11:32:21 UTC) #15
MikeSol
After large rebase
11 years, 2 months ago (2013-03-01 13:25:58 UTC) #16
dak
https://codereview.appspot.com/7424049/diff/19022/ly/spanners-init.ly File ly/spanners-init.ly (right): https://codereview.appspot.com/7424049/diff/19022/ly/spanners-init.ly#newcode114 ly/spanners-init.ly:114: breakSlur = #(make-music 'BreakSlurEvent) I am not happy with ...
11 years, 1 month ago (2013-03-08 12:54:38 UTC) #17
mike7
On 8 mars 2013, at 13:54, dak@gnu.org wrote: > > https://codereview.appspot.com/7424049/diff/19022/ly/spanners-init.ly > File ly/spanners-init.ly (right): ...
11 years, 1 month ago (2013-03-10 07:10:52 UTC) #18
MikeSol
Rebases against master
11 years, 1 month ago (2013-03-10 07:18:02 UTC) #19
lemzwerg
https://codereview.appspot.com/7424049/diff/41001/input/regression/repeat-slur.ly File input/regression/repeat-slur.ly (right): https://codereview.appspot.com/7424049/diff/41001/input/regression/repeat-slur.ly#newcode10 input/regression/repeat-slur.ly:10: " This should be rather @code{\\breakSlur}, @code{\\startBrokenSlur} and @code{\\stopBrokenSlur}. ...
11 years, 1 month ago (2013-03-18 17:52:16 UTC) #20
MikeSol
Uses \broken command for manually starting and ending repeat slurs.
11 years, 1 month ago (2013-03-19 06:06:49 UTC) #21
MikeSol
Adds copying of direction.
11 years, 1 month ago (2013-03-19 06:10:47 UTC) #22
MikeSol
The most recent patch set copies direction from SlurEvents and PhrasingSlurEvents, but this doesn't seem ...
11 years, 1 month ago (2013-03-19 06:13:33 UTC) #23
lemzwerg
LGTM. I can't help with the problem you are mentioning, but I have the feeling ...
11 years, 1 month ago (2013-03-19 07:01:01 UTC) #24
MikeSol
Correctly handles directions for broken slurs
11 years, 1 month ago (2013-03-19 14:19:58 UTC) #25
MikeSol
Adds comments.
11 years, 1 month ago (2013-03-19 14:36:10 UTC) #26
lemzwerg
https://codereview.appspot.com/7424049/diff/62001/input/regression/repeat-slur.ly File input/regression/repeat-slur.ly (right): https://codereview.appspot.com/7424049/diff/62001/input/regression/repeat-slur.ly#newcode9 input/regression/repeat-slur.ly:9: broken slur at a bar, use @code{\broken} with a ...
11 years, 1 month ago (2013-03-19 15:18:34 UTC) #27
MikeSol
Two backslashes
11 years, 1 month ago (2013-03-19 15:22:41 UTC) #28
dak
On 2013/03/19 15:22:41, MikeSol wrote: > Two backslashes After some consideration, I consider the name ...
11 years, 1 month ago (2013-03-19 21:26:27 UTC) #29
mike7
On 19 mars 2013, at 22:26, dak@gnu.org wrote: > On 2013/03/19 15:22:41, MikeSol wrote: >> ...
11 years, 1 month ago (2013-03-19 22:00:23 UTC) #30
wl_gnu.org
>> After some consideration, I consider the name \broken suboptimal since >> it implies two ...
11 years, 1 month ago (2013-03-19 22:37:14 UTC) #31
mike7
On 19 mars 2013, at 23:37, Werner LEMBERG <wl@gnu.org> wrote: >>> After some consideration, I ...
11 years, 1 month ago (2013-03-19 22:42:03 UTC) #32
dak
On 2013/03/19 22:37:14, wl_gnu.org wrote: > >> After some consideration, I consider the name \broken ...
11 years, 1 month ago (2013-03-20 02:24:23 UTC) #33
mike7
On 20 mars 2013, at 03:24, dak@gnu.org wrote: > On 2013/03/19 22:37:14, wl_gnu.org wrote: >> ...
11 years, 1 month ago (2013-03-20 04:23:12 UTC) #34
dak
"mike@mikesolomon.org" <mike@mikesolomon.org> writes: > Trying to put myself in the shoes of the average user, ...
11 years, 1 month ago (2013-03-20 05:07:31 UTC) #35
mike7
On 20 mars 2013, at 06:07, David Kastrup <dak@gnu.org> wrote: > "mike@mikesolomon.org" <mike@mikesolomon.org> writes: > ...
11 years, 1 month ago (2013-03-20 05:52:56 UTC) #36
wl_gnu.org
>> I vote for \broken. For me, it doesn't imply two pieces. This was >> ...
11 years, 1 month ago (2013-03-20 06:16:38 UTC) #37
wl_gnu.org
> Since \partial is already taken, what about \cut? The > spanner gets cut a ...
11 years, 1 month ago (2013-03-20 06:24:09 UTC) #38
wl_gnu.org
> I can even imagine to use to commands, \start and \end to get, say, ...
11 years, 1 month ago (2013-03-20 06:25:53 UTC) #39
mike7
On 20 mars 2013, at 07:25, Werner LEMBERG <wl@gnu.org> wrote: > >> I can even ...
11 years, 1 month ago (2013-03-20 06:34:47 UTC) #40
dak
"mike@mikesolomon.org" <mike@mikesolomon.org> writes: > On 20 mars 2013, at 06:07, David Kastrup <dak@gnu.org> wrote: > ...
11 years, 1 month ago (2013-03-20 06:50:31 UTC) #41
mike7
On 20 mars 2013, at 07:50, David Kastrup <dak@gnu.org> wrote: > "mike@mikesolomon.org" <mike@mikesolomon.org> writes: > ...
11 years, 1 month ago (2013-03-20 06:57:37 UTC) #42
t.daniels_treda.co.uk
mike@mikesolomon.org > I completely agree. It's just that "fake" in English means false or counterfeit. ...
11 years, 1 month ago (2013-03-20 08:26:02 UTC) #43
dak
"mike@mikesolomon.org" <mike@mikesolomon.org> writes: > On 20 mars 2013, at 07:50, David Kastrup <dak@gnu.org> wrote: > ...
11 years, 1 month ago (2013-03-20 08:41:18 UTC) #44
Ian Hulin (gmail)
\fake and \broken are concise but "feel" wrong, implying something's wrong with something else but ...
11 years, 1 month ago (2013-03-20 12:35:37 UTC) #45
pkx166h
On 20 March 2013 12:35, <ianhulin44@gmail.com> wrote: > \fake and \broken are concise but "feel" ...
11 years, 1 month ago (2013-03-20 13:16:51 UTC) #46
dak
On 2013/03/20 12:35:37, Ian Hulin (gmail) wrote: > \fake and \broken are concise but "feel" ...
11 years, 1 month ago (2013-03-20 13:22:21 UTC) #47
dak
Yet another possibility would be \inner but that implies a hierarchy. More accurate would be ...
11 years, 1 month ago (2013-03-20 13:26:01 UTC) #48
wl_gnu.org
> If the design needs it at both ends of the repeated block you could ...
11 years, 1 month ago (2013-03-20 13:27:14 UTC) #49
dak
And yet another would be \silent as a sort of negative variant of \visual.
11 years, 1 month ago (2013-03-20 13:28:50 UTC) #50
mike7
On 20 mars 2013, at 14:26, dak@gnu.org wrote: > Yet another possibility would be \inner ...
11 years, 1 month ago (2013-03-20 13:31:55 UTC) #51
wl_gnu.org
>> More accurate would be \continued. >> >> https://codereview.appspot.com/7424049/ > > I like \interrupted I ...
11 years, 1 month ago (2013-03-20 13:41:15 UTC) #52
pkx166h
Hello On 20 March 2013 13:40, Werner LEMBERG <wl@gnu.org> wrote: > >> More accurate would ...
11 years, 1 month ago (2013-03-20 13:46:56 UTC) #53
dak
"mike@mikesolomon.org" <mike@mikesolomon.org> writes: > On 20 mars 2013, at 14:26, dak@gnu.org wrote: > >> Yet ...
11 years, 1 month ago (2013-03-20 13:53:07 UTC) #54
dak
Werner LEMBERG <wl@gnu.org> writes: >> If the design needs it at both ends of the ...
11 years, 1 month ago (2013-03-20 13:53:13 UTC) #55
dak
James <pkx166h@gmail.com> writes: > Hello > > On 20 March 2013 13:40, Werner LEMBERG <wl@gnu.org> ...
11 years, 1 month ago (2013-03-20 13:58:53 UTC) #56
mike7
On 20 mars 2013, at 14:52, David Kastrup <dak@gnu.org> wrote: > "mike@mikesolomon.org" <mike@mikesolomon.org> writes: > ...
11 years, 1 month ago (2013-03-20 14:01:56 UTC) #57
wl_gnu.org
> I think any further proposals should _definitely_ explain how to write > the given ...
11 years, 1 month ago (2013-03-20 14:28:47 UTC) #58
x.scheuer_gmail.com
On 20 March 2013 15:01, mike@mikesolomon.org <mike@mikesolomon.org> wrote: > > I don't completely follow what ...
11 years, 1 month ago (2013-03-20 14:38:55 UTC) #59
dak
Werner LEMBERG <wl@gnu.org> writes: >> I think any further proposals should _definitely_ explain how to ...
11 years, 1 month ago (2013-03-20 15:38:54 UTC) #60
mike7
Sent from my iPhone On 20 mars 2013, at 16:38, David Kastrup <dak@gnu.org> wrote: > ...
11 years, 1 month ago (2013-03-20 17:25:10 UTC) #61
mike7
On 20 mars 2013, at 18:25, "mike@mikesolomon.org" <mike@mikesolomon.org> wrote: > Sent from my iPhone > ...
11 years, 1 month ago (2013-03-20 17:45:30 UTC) #62
dak
"mike@mikesolomon.org" <mike@mikesolomon.org> writes: > Even better, how about: > > \anderston > > The Anderston ...
11 years, 1 month ago (2013-03-20 18:00:28 UTC) #63
wl_gnu.org
>> But TeX is separate enough from LilyPond that I actually like >> \phantom rather ...
11 years, 1 month ago (2013-03-20 18:10:36 UTC) #64
mike7
On 20 mars 2013, at 09:26, Trevor Daniels <t.daniels@treda.co.uk> wrote: > > mike@mikesolomon.org > >> ...
11 years, 1 month ago (2013-03-20 18:34:21 UTC) #65
wl_gnu.org
> Even better, how about: > > \anderston Uhmm, no. :-) Werner
11 years, 1 month ago (2013-03-20 20:08:40 UTC) #66
dak
"mike@mikesolomon.org" <mike@mikesolomon.org> writes: > On 20 mars 2013, at 09:26, Trevor Daniels <t.daniels@treda.co.uk> wrote: > ...
11 years, 1 month ago (2013-03-20 22:51:38 UTC) #67
t.daniels_treda.co.uk
David Kastrup wrote Wednesday, March 20, 2013 10:38 PM > "mike@mikesolomon.org" <mike@mikesolomon.org> writes: > >> ...
11 years, 1 month ago (2013-03-20 23:23:27 UTC) #68
dak
"Trevor Daniels" <t.daniels@treda.co.uk> writes: > David Kastrup wrote Wednesday, March 20, 2013 10:38 PM > ...
11 years, 1 month ago (2013-03-20 23:45:10 UTC) #69
dak
David Kastrup <dak@gnu.org> writes: > "Trevor Daniels" <t.daniels@treda.co.uk> writes: > >>> What kind of word ...
11 years, 1 month ago (2013-03-20 23:57:06 UTC) #70
wl_gnu.org
> "At the start of the second alternative, we have an extra slur > start, ...
11 years, 1 month ago (2013-03-21 06:01:50 UTC) #71
Keith
Thanks for taking the time to do this. No-one else knows enough of the various ...
11 years, 1 month ago (2013-03-25 05:10:35 UTC) #72
mike7
On 25 mars 2013, at 07:10, k-ohara5a5a@oco.net wrote: > Thanks for taking the time to ...
11 years, 1 month ago (2013-03-25 08:40:08 UTC) #73
t.daniels_treda.co.uk
mike@mikesolomon.org wrote Monday, March 25, 2013 7:29 AM > On 25 mars 2013, at 07:10, ...
11 years, 1 month ago (2013-03-25 08:55:28 UTC) #74
Keith
On Mon, 25 Mar 2013 00:29:35 -0700, mike@mikesolomon.org <mike@mikesolomon.org> wrote: > On 25 mars 2013, ...
11 years, 1 month ago (2013-03-26 03:58:19 UTC) #75
mike7
On 26 mars 2013, at 05:58, "Keith OHara" <k-ohara5a5a@oco.net> wrote: > On Mon, 25 Mar ...
11 years, 1 month ago (2013-03-26 04:10:38 UTC) #76
MikeSol
Changes name, harmonizes slur direction
11 years, 1 month ago (2013-03-26 05:50:48 UTC) #77
Keith
Looks good. Gotta love monogamy.
11 years, 1 month ago (2013-03-26 08:17:45 UTC) #78
janek
Hi, please update the Rietveld issue description with current version of commit message. thanks, Janek
11 years, 1 month ago (2013-03-27 22:02:42 UTC) #79
Keith
On 2013/03/27 22:02:42, janek wrote: > current version of commit message. > I'll write a ...
11 years ago (2013-04-04 06:48:22 UTC) #80
janek
Hi Keith, On 2013/04/04 06:48:22, Keith wrote: > I'll write a commit message, just so ...
11 years ago (2013-04-07 22:43:37 UTC) #81
Keith
On 2013/03/26 04:10:38, mike7 wrote: > >> On 25 mars 2013, at 07:10, mailto:k-ohara5a5a@oco.net wrote: ...
11 years ago (2013-04-09 17:31:04 UTC) #82
mike7
11 years ago (2013-04-09 20:01:31 UTC) #83
On 9 avr. 2013, at 20:31, k-ohara5a5a@oco.net wrote:

> On 2013/03/26 04:10:38, mike7 wrote:
>> >> On 25 mars 2013, at 07:10, mailto:k-ohara5a5a@oco.net wrote:
>> >>
>> >>> Please do just one manual review of the regression suite between
>> >>> versions before adding another test of this length.
>> >>
>> I wanted to exhaust as many possibilities as I could think of (broken
> slurs over
>> one note, several notes, beginning broken, and broken, etc.) for both
> slur and
>> phrasing. Would it be better to split each system up into a separate
> test? Get
>> rid of some tests?
> 
> We need exhaustive test files while debugging, but probably we should
> reduce them to the essentials for the regtest that gets pushed.
> 
> Issue 3307 was plain to see in the regtest
> ‘rest-on-nonstandard-staff.ly’, but nobody succeeded in seeing the
> problem.
> 

Ok - feel free to chop this down however you see fit - I trust you.

Cheers,
MS

> https://codereview.appspot.com/7424049/
Sign in to reply to this message.

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