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

2010-02-20 Thread n . puttock

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

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


___
bug-lilypond mailing list
bug-lilypond@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-lilypond


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

2010-02-21 Thread ian

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




___
bug-lilypond mailing list
bug-lilypond@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-lilypond


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

2010-02-22 Thread Ian Hulin

On 20/02/10 17:27, n.putt...@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



___
bug-lilypond mailing list
bug-lilypond@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-lilypond


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

2010-02-22 Thread n . 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


___
bug-lilypond mailing list
bug-lilypond@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-lilypond


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

2010-02-22 Thread n . puttock

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

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


___
bug-lilypond mailing list
bug-lilypond@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-lilypond


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

2010-02-22 Thread ian


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

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


___
bug-lilypond mailing list
bug-lilypond@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-lilypond


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

2010-02-22 Thread Ian Hulin

Hi Neil,

Here's the amended patch.

Cheers,

Ian

On 22/02/10 22:17, n.putt...@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





>From ec0a781687878942995c3259b678fff17b58c666 Mon Sep 17 00:00:00 2001
From: Ian Hulin 
Date: Tue, 23 Feb 2010 00:17:21 +
Subject: [PATCH] T405 - Respect setting bracket-visibility property.

---
 lily/tuplet-bracket.cc |   47 +++
 1 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lily/tuplet-bracket.cc b/lily/tuplet-bracket.cc
index 76e249e..f525ef9 100644
--- a/lily/tuplet-bracket.cc
+++ b/lily/tuplet-bracket.cc
@@ -137,10 +137,10 @@ Tuplet_bracket::calc_connect_to_neighbors (SCM smob)
 {
   Spanner *me = unsmob_spanner (smob);
 
-  Direction dir = get_grob_direction (me); 
+  Direction dir = get_grob_direction (me);
   Drul_array bounds (get_x_bound_item (me, LEFT, dir),
 			 get_x_bound_item (me, RIGHT, dir));
-  
+
   Drul_array connect_to_other (false, false);
   Direction d = LEFT;
   do
@@ -169,11 +169,11 @@ Tuplet_bracket::calc_connect_to_neighbors (SCM smob)
   if (connect_to_other[LEFT] || connect_to_other[RIGHT])
 return scm_cons (scm_from_bool (connect_to_other[LEFT]),
 		 scm_from_bool (connect_to_other[RIGHT]));
-		 
+
   return SCM_EOL;
 }
 
-Grob * 
+Grob *
 Tuplet_bracket::get_common_x (Spanner *me)
 {
   extract_grob_set (me, "note-columns", columns);
@@ -282,16 +282,18 @@ Tuplet_bracket::print (SCM smob)
   bool equally_long = false;
   Grob *par_beam = parallel_beam (me, columns, &equally_long);
 
-  bool bracket_visibility = !(par_beam && equally_long);
+  bool bracket_visibility = !(par_beam && equally_long);  // Flag, print/don't print tuplet bracket.
   /*
-Fixme: the type of this prop is sucky.
+FIXME: The type of this prop is sucky.
   */
-  SCM bracket = me->get_property ("bracket-visibility");
-  if (scm_is_bool (bracket))
-bracket_visibility = ly_scm2bool (bracket);
-  else if (bracket == ly_symbol2scm ("if-no-beam"))
+  SCM bracket_vis_prop = me->get_property ("bracket-visibility");
+  bool bracket_prop = ly_scm2bool (bracket_vis_prop); // Flag, user has set bracket-visibility prop.
+  bool bracket = (bracket_vis_prop == ly_symbol2scm ("if-no-beam"));
+  if (scm_is_bool (bracket_vis_prop))
+bracket_visibility = bracket_prop;
+  else if (bracket)
 bracket_visibility = !par_beam;
-  
+
   /*
 Don't print a tuplet bracket and number if
 no control-points were calculated
@@ -303,13 +305,17 @@ Tuplet_bracket::print (SCM smob)
   return SCM_EOL;
 }
   /*  if the tuplet does not span any time, i.e. a single-note tuplet, hide
-  the bracket, but still let the number be displayed */
-  if (robust_scm2moment (me->get_bound (LEFT)->get_column ()->get_property ("when"), Moment (0))
-  == robust_scm2moment (me->get_bound (RIGHT)->get_column ()->get_property ("when"), Moment (0)))
+  the bracket, but still let the number be displayed.
+  Only do this if the user has not explicitly specified bracket-visibility = #t.
+  */
+  if (!bracket_prop)
   {
-  bracket_visibility = false;
+  if (robust_scm2moment (me->get_bound (LEFT)->get_column ()->get_property ("when"), Moment (0))
+	  == robust_scm2moment (me->get_bound (RIGHT)->get_column ()->get_property ("when"), Moment (0)))
+  {
+	  bracket_visibility = false;
+  }
   }
-
   Drul_array points;
   points[LEFT] = ly_scm2offset (scm_car (cpoints));
   points[RIGHT] = ly_scm2offset (scm_cadr (cpoints));
@@ -322,7 +328,8 @@ Tuplet_bracket::print (SCM smob)
   Grob *number_grob = unsmob_grob (me->get_object ("tuplet-number"));
 
   /*
-No bracket when it would be smaller than the number.
+Don't print the bracket when it would be smaller than the number.
+...Unless the user has coded bracket-visibility = #t, that is.
   */
   Real gap = 0.;
   if (bracket_visibility && number_grob)
@@ -332,7 +339,7 @@ Tuplet_bracket::print (SCM smob)
 	{
 	  gap = ext.length () + 1.0;
 
-	  if (0.75 * x_span.length () < gap)
+	  if ((0.75 * x_span.length () < gap) && !bracket_prop)
 	bracket_visibility = false;
 	}
 }
@@ -621,7 +628,7 @@ Tuplet_bracket::calc_position_and_height (Grob *me_grob, Real *offset, Real *dy)
   points.push_back (Offset (x0 - x0, staff[dir]));
   points.push_back (Offset (x1 - x0, staff[dir]));
 }
-  
+
   /*
 This is a slight hack. We compute two encompass points from the
 bbox of the smaller tuplets.
@@ -679,7 +686,7 @@ Tuplet_bracket::calc_po