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

Issue 315350043: Implement shorten-pair for Hairpin

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 2 months ago by david.nalesnik
Modified:
7 years, 2 months ago
Reviewers:
thomasmorley651
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Implement shorten-pair for Hairpin This property allows the user to offset the ends of hairpins independently.

Patch Set 1 #

Patch Set 2 : documentation, regtests #

Total comments: 2

Patch Set 3 : improve snippet #

Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -4 lines) Patch
M Documentation/changes.tely View 1 1 chunk +14 lines, -0 lines 0 comments Download
M Documentation/notation/expressive.itely View 1 1 chunk +3 lines, -0 lines 0 comments Download
A Documentation/snippets/new/moving-the-ends-of-hairpins.ly View 1 2 1 chunk +30 lines, -0 lines 0 comments Download
A input/regression/hairpin-shorten-pair.ly View 1 1 chunk +26 lines, -0 lines 0 comments Download
A input/regression/hairpin-shorten-pair-circled-tip.ly View 1 1 chunk +22 lines, -0 lines 0 comments Download
M lily/hairpin.cc View 4 chunks +9 lines, -1 line 0 comments Download
M scm/define-grob-properties.scm View 1 1 chunk +4 lines, -3 lines 0 comments Download

Messages

Total messages: 8
david.nalesnik
Please review. Thanks!
7 years, 2 months ago (2017-01-07 17:12:57 UTC) #1
thomasmorley651
On 2017/01/07 17:12:57, david.nalesnik wrote: > Please review. Thanks! Hi David, very nice. I can't ...
7 years, 2 months ago (2017-01-08 11:37:18 UTC) #2
david.nalesnik
On 2017/01/08 11:37:18, thomasmorley651 wrote: > On 2017/01/07 17:12:57, david.nalesnik wrote: > > Please review. ...
7 years, 2 months ago (2017-01-08 12:48:33 UTC) #3
david.nalesnik
On 2017/01/08 11:37:18, thomasmorley651 wrote: > On 2017/01/07 17:12:57, david.nalesnik wrote: > > Please review. ...
7 years, 2 months ago (2017-01-14 21:03:43 UTC) #4
thomasmorley651
Again limited to the scm/ly-files: LGTM One nit: https://codereview.appspot.com/315350043/diff/20001/Documentation/snippets/new/moving-the-ends-of-hairpins.ly File Documentation/snippets/new/moving-the-ends-of-hairpins.ly (right): https://codereview.appspot.com/315350043/diff/20001/Documentation/snippets/new/moving-the-ends-of-hairpins.ly#newcode18 Documentation/snippets/new/moving-the-ends-of-hairpins.ly:18: c'1~\< ...
7 years, 2 months ago (2017-01-14 21:53:25 UTC) #5
david.nalesnik
improve snippet
7 years, 2 months ago (2017-01-15 17:05:14 UTC) #6
david.nalesnik
https://codereview.appspot.com/315350043/diff/20001/Documentation/snippets/new/moving-the-ends-of-hairpins.ly File Documentation/snippets/new/moving-the-ends-of-hairpins.ly (right): https://codereview.appspot.com/315350043/diff/20001/Documentation/snippets/new/moving-the-ends-of-hairpins.ly#newcode18 Documentation/snippets/new/moving-the-ends-of-hairpins.ly:18: c'1~\< On 2017/01/14 21:53:25, thomasmorley651 wrote: > I'd suggest ...
7 years, 2 months ago (2017-01-15 17:07:57 UTC) #7
thomasmorley651
7 years, 2 months ago (2017-01-15 17:28:38 UTC) #8
On 2017/01/15 17:07:57, david.nalesnik wrote:
>
https://codereview.appspot.com/315350043/diff/20001/Documentation/snippets/ne...
> File Documentation/snippets/new/moving-the-ends-of-hairpins.ly (right):
> 
>
https://codereview.appspot.com/315350043/diff/20001/Documentation/snippets/ne...
> Documentation/snippets/new/moving-the-ends-of-hairpins.ly:18: c'1~\<
> On 2017/01/14 21:53:25, thomasmorley651 wrote:
> > I'd suggest to make it more clear what happens 
> > if the tweaked hairpin spans between DynamicText like
> >   c'1~\p\< c'2~ c'\ffff\!
> > Maybe extend the 'hairpin'-example a bit or drop an additional line in the
> > texidoc-header about affecting the _visible_ length of the Hairpin, opposed
to
> > the behaviour of 'minimum-length
> 
> Done.

Thanks.

LGTM
Sign in to reply to this message.

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