Issue 6490043: Ledger-line-spanner: symmetric extents; issue 2493

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 6490043: Ledger-line-spanner: symmetric extents; issue 2493 (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years, 8 months ago by Keith

Modified:
11 years, 4 months ago

Reviewers:
MikeSol

CC:
lilypond-devel_gnu.org

Visibility:
Public.

Description

Ledger-line-spanner: symmetric extents; issue 2493

Patch Set 1 : use real note-head width #

Total comments: 1

Patch Set 2 : check column order #

Patch Set 3 : modify regtests #

Total comments: 2

Patch Set 4 : just skip ambitus heads #

Created: 11 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+16 lines, -11 lines)			Patch
M	input/regression/compound-time-signatures.ly	View	1 2 3	1 chunk	+1 line, -0 lines	0 comments	Download
M	input/regression/tablature-full-notation.ly	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download
M	lily/ledger-line-spanner.cc	View	1 2 3	6 chunks	+13 lines, -11 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

Keith

http://codereview.appspot.com/6490043/diff/2001/lily/ledger-line-spanner.cc File lily/ledger-line-spanner.cc (left): http://codereview.appspot.com/6490043/diff/2001/lily/ledger-line-spanner.cc#oldcode56 lily/ledger-line-spanner.cc:56: * (3 / 2 * min_length_fraction) The 3/2 might ...

11 years, 8 months ago (2012-08-28 09:20:19 UTC) #1

Keith

http://codereview.appspot.com/6490043/diff/7001/input/regression/compound-time-signatures.ly File input/regression/compound-time-signatures.ly (right): http://codereview.appspot.com/6490043/diff/7001/input/regression/compound-time-signatures.ly#newcode19 input/regression/compound-time-signatures.ly:19: \override Staff.TimeSignature #'break-visibility = #'#(#f #f #t) (#f #t ...

11 years, 8 months ago (2012-08-29 05:15:45 UTC) #2

MikeSol

11 years, 8 months ago (2012-08-29 18:23:42 UTC) #3

http://codereview.appspot.com/6490043/diff/7001/lily/ledger-line-spanner.cc
File lily/ledger-line-spanner.cc (right):

http://codereview.appspot.com/6490043/diff/7001/lily/ledger-line-spanner.cc#n...
lily/ledger-line-spanner.cc:50: < Paper_column::get_rank (previous_column)))
I'm having trouble following this naming convention.  It seems that
previous_column should have a rank falling before current column, not after.

Also, it seems like if the variables are called this, then the order should be
correct when they are passed into the function (meaning the one supposed to come
before the other comes before the other).  Otherwise a programming error should
be raised.  What would be the case where the ranks of the columns would be
reversed and what would justify that happening?

Expand All Messages | Collapse All Messages