[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Add independent control of thickness and offset for underline markup
Re: Add independent control of thickness and offset for underline markup (issue1347041)
Sat, 29 May 2010 11:35:18 -0600
On 5/29/10 10:10 AM, "Kieren MacMillan" <address@hidden>
> Hi Neil,
> Thanks for the comments!
> [Should I be somehow commenting on codereview instead of here?]
Either way works.
>> I don't think we can remove the link between 'line-thickness and
>> underline offset, since it should scale based on staff-size. At small
>> staff-sizes, 'line-thickness gets progressively larger, which matches
>> the thicker underline with a slightly bigger gap.
> OK. But I want two things out of this patch:
> 1. Control of offset independent of thickness, even if offset is defined
> as a "multiple of 'line-thickness" (that I can adjust).
> 2. An easy way to globally set the property (i.e., not just a direct
> \override of the \underline command).
>> What you could do instead is leave out the default value for offset and
>> keep the existing behaviour unless the user sets offset (if it's unset,
>> it will return #f).
> I tried
> #:properties ((thickness 1))
> and it failed.
>> scm/define-markup-commands.scm:280: #:properties ((thickness 1) (offset
>> I think this default for offset is too large.
> This is precisely why we need the offset property... ;)
>> scm/define-markup-commands.scm:301: (y (* -1 offset))
>> (- offset)
>> Couldn't we define the offset as a multiple of 'line-thickness, instead of
>> an absolute value? That's how fret diagrams are scaled.
> Sounds good to me!
> To be clear, 'line-thickness and the thickness property of \underline are
> [potentially] two different independent values, right?
Yes. I would suggest that both thickness and offset should be multipliers
of 'line-thickness (which shows up in your scheme code as thick.
> Once these points are addressed, how do I submit a revision?
Attach (or inline) a diff patch on an email, and I'll apply and repost to