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

Issue 4188051: Remove special case in staff-spacing (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 1 month ago by Keith
Modified:
12 years, 7 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Patch set 3; issue 1779 : Avoid redundant space in note spacing ly/staff-spacing.cc: if clearances would be tight iff the line is compressed, leave the ideal spacing as-is, for use in case there is ample room. ly/spacing-spanner.cc: compute total spring length before checking clearances. Patch set 4 adds (issue 1785) : Restore the effect of fixed spacing in space-alist

Patch Set 1 : with prefatory space #

Patch Set 2 : handle first-measure-extra-space as well #

Total comments: 1

Patch Set 3 : Simpler arrangement #

Patch Set 4 : Restore 'fixed' effect in space-alist #

Total comments: 7

Patch Set 5 : Restore TimeSignature extra space #

Patch Set 6 : pass full-measure-extra-space to staff-spacing #

Total comments: 1

Patch Set 7 : rebased to tab-free codebase #

Patch Set 8 : as pushed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -27 lines) Patch
M input/regression/spacing-bar-accidental.ly View 1 2 1 chunk +1 line, -1 line 0 comments Download
M input/regression/tablature-full-notation.ly View 1 2 3 4 5 6 7 1 chunk +1 line, -2 lines 0 comments Download
M lily/include/staff-spacing.hh View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M lily/spacing-determine-loose-columns.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M lily/spacing-spanner.cc View 1 2 3 4 5 6 3 chunks +8 lines, -10 lines 0 comments Download
M lily/staff-spacing.cc View 1 2 3 4 5 6 7 2 chunks +14 lines, -6 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 7 chunks +7 lines, -6 lines 0 comments Download

Messages

Total messages: 16
pkx166h
Passes make and there are lots (lots) of reg test differences but as keith says ...
12 years, 8 months ago (2011-07-25 20:27:03 UTC) #1
joeneeman
http://codereview.appspot.com/4188051/diff/4001/lily/include/spacing-spanner.hh File lily/include/spacing-spanner.hh (right): http://codereview.appspot.com/4188051/diff/4001/lily/include/spacing-spanner.hh#newcode45 lily/include/spacing-spanner.hh:45: static Real cushion_; I think this should be a ...
12 years, 8 months ago (2011-07-26 02:27:07 UTC) #2
Keith
>> lily/include/spacing-spanner.hh:45: static Real cushion_; > I think this should be a grob property rather ...
12 years, 8 months ago (2011-07-26 04:30:44 UTC) #3
Keith
> http://codereview.appspot.com/4188051/diff/4001/lily/include/spacing-spanner.hh#newcode45 > lily/include/spacing-spanner.hh:45: static Real cushion_; > I think this should be a grob ...
12 years, 7 months ago (2011-07-28 00:16:47 UTC) #4
Keith
As a separate commit, I should probably restore the uncompressibility of the space after prefatory ...
12 years, 7 months ago (2011-07-28 01:26:48 UTC) #5
Janek Warchol
Keith, I am so sorry - you've sent me this patch ages ago and i ...
12 years, 7 months ago (2011-07-29 04:31:00 UTC) #6
Keith
On 2011/07/29 04:31:00, Janek Warchol wrote: > Could you add some comments for rookies like ...
12 years, 7 months ago (2011-07-29 05:31:14 UTC) #7
Keith
http://codereview.appspot.com/4188051/diff/17001/scm/define-grobs.scm File scm/define-grobs.scm (left): http://codereview.appspot.com/4188051/diff/17001/scm/define-grobs.scm#oldcode2237 scm/define-grobs.scm:2237: (extra-spacing-height . (-1.0 . 1.0)) On 2011/07/29 05:31:15, Keith ...
12 years, 7 months ago (2011-07-29 06:50:19 UTC) #8
Janek Warchol
2011/7/29 <k-ohara5a5a@oco.net>: > On 2011/07/29 04:31:00, Janek Warchol wrote: >> >> Could you add some ...
12 years, 7 months ago (2011-07-30 15:18:58 UTC) #9
Keith
On 2011/07/30 15:18:58, Janek Warchol wrote: > 2011/7/29 <k-ohara5a5a@oco.net>: > > > > The old ...
12 years, 7 months ago (2011-07-31 03:47:18 UTC) #10
Janek Warchol
Keith, sorry for the delayed answer, i had to sort out my repository because of ...
12 years, 7 months ago (2011-08-02 21:29:24 UTC) #11
Keith
On 2011/08/02 21:29:24, Janek Warchol wrote: > It does the things i hoped for for ...
12 years, 7 months ago (2011-08-03 07:16:08 UTC) #12
Janek Warchol
2011/8/3 <k-ohara5a5a@oco.net>: > On 2011/08/02 21:29:24, Janek Warchol wrote: > >> It does the things ...
12 years, 7 months ago (2011-08-04 22:02:22 UTC) #13
Janek Warchol
After thinking and thinking, i think i know now what's the answer :) 2011/8/5 Janek ...
12 years, 7 months ago (2011-08-07 08:13:40 UTC) #14
MikeSol
LGTM. Cheers, MS
12 years, 7 months ago (2011-08-21 16:23:23 UTC) #15
janek
12 years, 7 months ago (2011-08-22 16:08:16 UTC) #16
Keith,

can you close Rietveld issue 4188051?

thanks,
Janek
Sign in to reply to this message.

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