[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Updated patch for issue 155 (issue 230860044 by address@hidden)
From: |
k-ohara5a5a |
Subject: |
Re: Updated patch for issue 155 (issue 230860044 by address@hidden) |
Date: |
Sun, 19 Apr 2015 19:32:29 +0000 |
LGTM.
The main difference is the change in handling pure functions.
I can't see any difference here relative to Joe's patch, but it looks
fine.
https://codereview.appspot.com/230860044/diff/20001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):
https://codereview.appspot.com/230860044/diff/20001/scm/define-grob-properties.scm#newcode777
scm/define-grob-properties.scm:777: (parenthesis-friends ,list? "A lisy
of symbols. Any parenthesis
"A list of Grob types, as symbols. When parentheses enclose a Grob that
has 'parenthesis-friends, the parentheses widen to include any child
Grobs with type among 'parethesis-friends."
https://codereview.appspot.com/230860044/diff/20001/scm/define-grob-properties.scm#newcode779
scm/define-grob-properties.scm:779: these symbols on the parent grob.")
I think 'this grob' and 'parent grob' are the same, but I was confused,
so I tried to rewrite what I hope is simpler and correct.
https://codereview.appspot.com/230860044/diff/20001/scm/output-lib.scm
File scm/output-lib.scm (right):
https://codereview.appspot.com/230860044/diff/20001/scm/output-lib.scm#newcode908
scm/output-lib.scm:908: (define-public (parentheses-item::pure-y-extent
grob start end)
I do not see why this function is necessary, but it looks correct.
There is a distinction between pure-y-extent and y-extent for objects
whose vertical position or height might change depending on where the
line-breaks fall.
Maybe it was formerly necessary for the accidental on the second note of
a tie, which might disappear, but we let those accidentals have a simple
height now.
The only need I could see for pure-y-extent here, would be to estimate
horizontal space for the parentheses around an accent outside a slur or
beam.
https://codereview.appspot.com/230860044/