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

Issue 1670042: Fix 1112. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 10 months ago by joeneeman
Modified:
13 years, 8 months ago
Reviewers:
Neil Puttock
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fix 1112. Add support for minimum-distance into the page-breaker.

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -29 lines) Patch
A input/regression/page-breaking-min-distance.ly View 1 chunk +18 lines, -0 lines 3 comments Download
M lily/constrained-breaking.cc View 5 chunks +30 lines, -3 lines 4 comments Download
M lily/include/constrained-breaking.hh View 5 chunks +12 lines, -1 line 0 comments Download
M lily/page-breaking.cc View 3 chunks +31 lines, -20 lines 0 comments Download
M lily/page-spacing.cc View 2 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 3
Neil Puttock
LGTM. http://codereview.appspot.com/1670042/diff/1/2 File input/regression/page-breaking-min-distance.ly (right): http://codereview.appspot.com/1670042/diff/1/2#newcode1 input/regression/page-breaking-min-distance.ly:1: \version "2.13.22" 2.13.26 http://codereview.appspot.com/1670042/diff/1/2#newcode9 input/regression/page-breaking-min-distance.ly:9: between-scores-system-spacing #'minimum-distance = ...
13 years, 10 months ago (2010-06-20 20:12:52 UTC) #1
joeneeman
Thanks, fixed. I'll push after "make check" finishes... http://codereview.appspot.com/1670042/diff/1/3 File lily/constrained-breaking.cc (right): http://codereview.appspot.com/1670042/diff/1/3#newcode384 lily/constrained-breaking.cc:384: Line_details ...
13 years, 10 months ago (2010-06-21 18:28:52 UTC) #2
Neil Puttock
13 years, 10 months ago (2010-06-22 21:21:16 UTC) #3
On 2010/06/21 18:28:52, joeneeman wrote:

> We read it in several places, but we never set it to anything other than zero.
> So it's ready to be used as soon as we decide to set it to something better
than
> zero, but I don't know what that is yet...

Ah, I see.  So the same applies to between_system_space_ in
Constrained_breaking.
 
> It's read in lines 413 and 425 (but I see that it isn't documented correctly
in
> the NR...)

It doesn't look very useful. :)

Cheers,
Neil
Sign in to reply to this message.

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