T405 - Respect user setting bracket-visibility property.

T405 - Respect user setting bracket-visibility property.

[Neil Puttock, 2010-02-20 17:27:56]

Hi Ian,

Looks OK, though don't you think it would be better to increase the size of the
bracket if it's too small?

Cheers,
Neil

http://codereview.appspot.com/194095/diff/1/2
File lily/tuplet-bracket.cc (right):

http://codereview.appspot.com/194095/diff/1/2#newcode286
lily/tuplet-bracket.cc:286: SCM bracket_vis_prop = me->get_property
("bracket-visibility"); // The type of this prop is sucky.
Unless you're going to fix this, you shouldn't remove the FIXME

[Ian Hulin, 2010-02-21 12:34:25]

Hi Neil,
I've fixed the comment.  Please push this patch (I've rebased it to branch from
latest origin/master).

Cheers,

Ian

http://codereview.appspot.com/194095/diff/1/2
File lily/tuplet-bracket.cc (right):

http://codereview.appspot.com/194095/diff/1/2#newcode286
lily/tuplet-bracket.cc:286: SCM bracket_vis_prop = me->get_property
("bracket-visibility"); // The type of this prop is sucky.
On 2010/02/20 17:27:56, Neil Puttock wrote:
> Unless you're going to fix this, you shouldn't remove the FIXME

Done.
Restored the block comment and the FIXME:

[Ian Hulin, 2010-02-22 18:27:19]

On 20/02/10 17:27, n.puttock@gmail.com wrote:
> Hi Ian,
>
> Looks OK, though don't you think it would be better to increase the size
> of the bracket if it's too small?
>
We--ll, yes, but that's not what this bug report is about.  As it's 
something you've picked up during the review it's strictly speaking a 
new issue.

Sorry if this sounds a bit jobsworth but I'd rather this patch went out 
the door as is and I'll look at your comment as part of work on a new 
tracker.

Cheers,

Ian

[Neil Puttock, 2010-02-22 22:17:07]

On 2010/02/22 18:27:19, Ian Hulin wrote:

> Sorry if this sounds a bit jobsworth but I'd rather this patch went out 
> the door as is and I'll look at your comment as part of work on a new 
> tracker.

Fair enough.  Send me the patch when you've sorted the nitpicks below.

Cheers,
Neil

[Neil Puttock, 2010-02-22 22:18:10]

Sorry, here they are:

http://codereview.appspot.com/194095/diff/2001/1003
File lily/tuplet-bracket.cc (right):

http://codereview.appspot.com/194095/diff/2001/1003#newcode4
lily/tuplet-bracket.cc:4: Copyright (C) 1997--2009 Jan Nieuwenhuizen
<janneke@gnu.org>
rebase again?

http://codereview.appspot.com/194095/diff/2001/1003#newcode287
lily/tuplet-bracket.cc:287: FIXME: The type of this prop is sucky.
indent

[Ian Hulin, 2010-02-22 23:43:41]

http://codereview.appspot.com/194095/diff/2001/1003
File lily/tuplet-bracket.cc (right):

http://codereview.appspot.com/194095/diff/2001/1003#newcode4
lily/tuplet-bracket.cc:4: Copyright (C) 1997--2009 Jan Nieuwenhuizen
<janneke@gnu.org>
On 2010/02/22 22:18:10, Neil Puttock wrote:
> rebase again?

Done.

http://codereview.appspot.com/194095/diff/2001/1003#newcode287
lily/tuplet-bracket.cc:287: FIXME: The type of this prop is sucky.
On 2010/02/22 22:18:10, Neil Puttock wrote:
> indent

Done.

[Ian Hulin, 2010-02-23 00:23:20]

Hi Neil,

Here's the amended patch.

Cheers,

Ian

On 22/02/10 22:17, n.puttock@gmail.com wrote:
> On 2010/02/22 18:27:19, Ian Hulin wrote:
>
>> Sorry if this sounds a bit jobsworth but I'd rather this patch went
> out
>> the door as is and I'll look at your comment as part of work on a new
>> tracker.
>
> Fair enough.  Send me the patch when you've sorted the nitpicks below.
>
> Cheers,
> Neil
>
> http://codereview.appspot.com/194095/show
>
> ---
> ----
> Join the Frogs!
>
>
> ______________________________________________       This email has 
> been scanned by Netintelligence       
> http://www.netintelligence.com/email
>
>