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

Issue 324800043: Improve elbowed-hairpin (Closed)

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

Description

Improve elbowed-hairpin Let the lines be printed by the new make-connected-line-procedure, using ly:line-interface::line. The new stencil now reacts on overrides for style and dash-period/fraction. Not closing Hairpins created by elbowed-hairpin are possible now. Single disadvantage: The point-list needs to have (0 . 0) first, if a closing Hairpin is wished. The previous used make-connected-path did that automatically and thus was not flexible enough. Give the final stencil proper extents. Cleanup, more descriptive naming, extent docstring.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Adressing David N's comments, extend docstring #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -32 lines) Patch
M scm/output-lib.scm View 1 2 chunks +64 lines, -32 lines 2 comments Download

Messages

Total messages: 6
thomasmorley651
Please review.
6 years, 11 months ago (2017-04-22 23:09:50 UTC) #1
david.nalesnik
LGTM. I've pointed out two minor issues, but I don't believe they should hold up ...
6 years, 11 months ago (2017-04-24 14:42:18 UTC) #2
thomasmorley651
Adressing David N's comments, extend docstring
6 years, 11 months ago (2017-04-24 23:14:17 UTC) #3
thomasmorley651
On 2017/04/24 14:42:18, david.nalesnik wrote: > LGTM. > > I've pointed out two minor issues, ...
6 years, 11 months ago (2017-04-24 23:24:30 UTC) #4
david.nalesnik
LGTM. (Aside from docstring change which you could make when pushing.) https://codereview.appspot.com/324800043/diff/20001/scm/output-lib.scm File scm/output-lib.scm (right): ...
6 years, 11 months ago (2017-04-25 15:06:22 UTC) #5
thomasmorley651
6 years, 11 months ago (2017-04-25 22:56:35 UTC) #6
https://codereview.appspot.com/324800043/diff/20001/scm/output-lib.scm
File scm/output-lib.scm (right):

https://codereview.appspot.com/324800043/diff/20001/scm/output-lib.scm#newcod...
scm/output-lib.scm:1170: Returns a line connecting @var{pts}, using
@code{ly:line-interface::line}, gets
On 2017/04/25 15:06:21, david.nalesnik wrote:
> Should be @var{points} now.

Done.
Though, this doesn't need a new patch-set.
Local tests succeeded.
Sign in to reply to this message.

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