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

Issue 295970043: Allow override of NoteHead.ledger-positions (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 9 months ago by pwm
Modified:
3 years, 8 months ago
Reviewers:
dak, lemzwerg
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Allow override of NoteHead.ledger-positions Can be a literal list of positions: \once \override NoteHead.ledger-positions = #'(...) Or a scheme procedure that returns such a list: \override NoteHead.ledger-positions = #(lambda (grob) ...)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Revision based on David's feedback #

Patch Set 3 : Add StaffSymbol.ledger-positions-function #

Total comments: 2

Patch Set 4 : some minor cleanups #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -6 lines) Patch
A input/regression/ledger-positions-customization.ly View 1 2 3 1 chunk +27 lines, -0 lines 0 comments Download
M lily/include/staff-symbol.hh View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/ledger-line-spanner.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M lily/note-head.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/staff-symbol.cc View 1 2 3 3 chunks +22 lines, -2 lines 0 comments Download
M scm/define-grob-properties.scm View 1 2 1 chunk +7 lines, -2 lines 0 comments Download

Messages

Total messages: 9
pwm
Please review. I've posted an implementation question in the issue tracker. https://sourceforge.net/p/testlilyissues/issues/4828/ Thanks, -Paul
3 years, 9 months ago (2016-04-20 13:29:54 UTC) #1
dak
https://codereview.appspot.com/295970043/diff/1/lily/ledger-line-spanner.cc File lily/ledger-line-spanner.cc (right): https://codereview.appspot.com/295970043/diff/1/lily/ledger-line-spanner.cc#newcode221 lily/ledger-line-spanner.cc:221: int len = scm_to_int (scm_length (posns)); scm_ilength already does ...
3 years, 9 months ago (2016-04-22 08:17:09 UTC) #2
pwm
Revision based on David's feedback
3 years, 9 months ago (2016-04-23 04:31:28 UTC) #3
pwm
Patch set 2, based on David's Feedback. -Paul https://codereview.appspot.com/295970043/diff/1/lily/ledger-line-spanner.cc File lily/ledger-line-spanner.cc (right): https://codereview.appspot.com/295970043/diff/1/lily/ledger-line-spanner.cc#newcode221 lily/ledger-line-spanner.cc:221: int ...
3 years, 9 months ago (2016-04-23 04:34:44 UTC) #4
pwm
Add StaffSymbol.ledger-positions-function
3 years, 9 months ago (2016-04-23 20:33:21 UTC) #5
pwm
On 2016/04/23 20:33:21, pwm wrote: > Add StaffSymbol.ledger-positions-function I realized that calls to StaffSymbol::on_line would ...
3 years, 9 months ago (2016-04-23 20:48:19 UTC) #6
lemzwerg
LGTM. https://codereview.appspot.com/295970043/diff/40001/input/regression/ledger-positions-customization.ly File input/regression/ledger-positions-customization.ly (right): https://codereview.appspot.com/295970043/diff/40001/input/regression/ledger-positions-customization.ly#newcode19 input/regression/ledger-positions-customization.ly:19: #'(lambda (staff-symbol-grob pos) (list pos)) I would indent ...
3 years, 9 months ago (2016-04-24 03:05:12 UTC) #7
pwm
some minor cleanups
3 years, 9 months ago (2016-04-25 03:05:17 UTC) #8
pwm
3 years, 9 months ago (2016-04-25 03:12:55 UTC) #9
Patch set 4 with some minor cleanups.

https://codereview.appspot.com/295970043/diff/40001/input/regression/ledger-p...
File input/regression/ledger-positions-customization.ly (right):

https://codereview.appspot.com/295970043/diff/40001/input/regression/ledger-p...
input/regression/ledger-positions-customization.ly:19: #'(lambda
(staff-symbol-grob pos) (list pos))
On 2016/04/24 03:05:12, lemzwerg wrote:
> I would indent this line...

Thanks for catching this.  Done, along with a few other other cleanups and
making things more concise in staff-symbol.cc, in patch set 4.
Sign in to reply to this message.

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