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

Issue 575330043: add property label-alignments to OttavaBracket

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 2 days ago by Malte Meyn
Modified:
1 week, 3 days ago
Reviewers:
thomasmorley651, lemzwerg, carl.d.sorensen, c_sorensen
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

add property label-alignments to OttavaBracket

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -2 lines) Patch
M lily/ottava-bracket.cc View 2 chunks +18 lines, -2 lines 0 comments Download
M scm/define-grob-properties.scm View 1 chunk +4 lines, -0 lines 1 comment Download
M scm/define-grobs.scm View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 7
lemzwerg
Very nice, LGTM!
2 weeks, 2 days ago (2019-11-21 13:48:37 UTC) #1
Carl
I've added a comment about the docstring -- it's a request for consideration, not a ...
2 weeks, 2 days ago (2019-11-21 14:29:35 UTC) #2
Malte Meyn
Hm, I think you’re right. I’m not sure how interface work but maybe this is ...
2 weeks, 1 day ago (2019-11-22 12:56:00 UTC) #3
thomasmorley651
What about a regtest?
2 weeks, 1 day ago (2019-11-22 20:34:32 UTC) #4
Malte Meyn
On 2019/11/22 12:56:00, Malte Meyn wrote: > I just managed to do the same thing ...
2 weeks ago (2019-11-23 11:18:10 UTC) #5
Malte Meyn
On 2019/11/23 11:18:10, Malte Meyn wrote: > I would suggest to: > • add the ...
1 week, 3 days ago (2019-11-27 05:40:16 UTC) #6
c_sorensen
1 week, 3 days ago (2019-11-27 05:52:27 UTC) #7

On 11/26/19, 10:40 PM, "lilypond@maltemeyn.de" <lilypond@maltemeyn.de> wrote:

    On 2019/11/23 11:18:10, Malte Meyn wrote:
    > I would suggest to:
    > • add the label-alignments property to all spanners with exactly one
    label
    > • think about all those different piano pedal grobs, whether they are
    needed,
    > and how alignment could be done between the label grob and the line
    grob
    > • rename stencil-align-dir-y to label-alignments and let it take a
    pair of
    > numbers (is this good, to have a top-level property for some grobs
    with the same
    > name as a subproperty for others?)
    
    Any opinions, helpful thoughts?

I don't know if it's helpful, but if it provides the same functionality, I think
it should have the same name, regardless of whether it's a top-level property or
subproperty.

I think it's great that you are taking on the bigger picture.  My initial
comment was not trying to get you to take on the bigger picture; it was just to
make sure that the docstring for label-alignments wasn't limited to
OttavaBrackets.  Considering other spanners and trying to rationalize all the
labeling is a great contribution, beyond my expectation.

Thanks,

Carl

    
    https://codereview.appspot.com/575330043/
    

Sign in to reply to this message.

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