Re: T405 - Respect user setting bracket-visibility property. (issue194095)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: T405 - Respect user setting bracket-visibility property. (issue194095)

Ian Hulin
Reviewers: Neil Puttock,

Message:
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:

Description:
T405 - Respect user setting bracket-visibility property.

Please review this at http://codereview.appspot.com/194095/show

Affected files:
   M lily/tuplet-bracket.cc



---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: T405 - Respect user setting bracket-visibility property. (issue194095)

Neil Puttock
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!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: T405 - Respect user setting bracket-visibility property. (issue194095)

Neil Puttock
In reply to this post by Ian Hulin
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
<[hidden email]>
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

http://codereview.appspot.com/194095/show

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: T405 - Respect user setting bracket-visibility property. (issue194095)

Ian Hulin
In reply to this post by Ian Hulin

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
<[hidden email]>
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.

http://codereview.appspot.com/194095/show

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: T405 - Respect user setting bracket-visibility property. (issue194095)

Ian Hulin
In reply to this post by Neil Puttock
Hi Neil,

Here's the amended patch.

Cheers,

Ian

On 22/02/10 22:17, [hidden email] 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
>
>


0001-T405-Respect-setting-bracket-visibility-property.patch (4K) Download Attachment
Loading...