Re: get rid of minimum-[XY]-extents (issue 6294086)

[janek . lilypond]

Reviewers: Keith, david.nalesnik,

Message:
Hi Keith,
David (i'd like to know your opinion about something here)
and all,

On 2012/06/19 08:53:44, Keith wrote:
> Nobody told me minimum-*-extents were deprecated, except for their use
in
> staff-spacing,

You're right, i was imprecise: they are deprecated in staff-spacing and
virtually unused in the whole codebase.

> so I have been using and recommending them.
> http://lists.gnu.org/archive/html/lilypond-user/2011-11/msg00217.html
> http://lists.gnu.org/archive/html/lilypond-user/2011-07/msg00427.html

> The docstring for X-extent says it is "hard-coded" so I didn't think I
was
> allowed to change them.

Indeed that description looks wrong.

> The minimum-*-extent is convenient when you want extra
> space on one side, and don't want to think about what
> extent the object should have on the other side.

Indeed they can be handy in situations like that.
However, i think that we should approach this problem differently,
in a more flexible and less hard-coded way.

Here's the downside of minimum-*-extent solution: what if i need to
specify extent's upper bound instead of lower one?  There's no
"maximum-[XY]-extent" property.  What if i want to restrict [XY]-offset?
Again, [minimum|maximum]-[XY]-offset properties don't exist.
So, should we create them for all similar properties?
Surely not, that would be a hell lot of boilerplate code!

I think that we should use scheme functions for things like that.
I'm pretty sure that one can easily write a scheme function
which would set [XY]-extent to at least something.
Or a funtion that would modify only one element in a pair.
David, could you confirm that it's possible to easily define
such funtions?

Keith, for an illustration of such "modifying functions" see this
email from David Nalesnik:
http://lists.gnu.org/archive/html/lilypond-devel/2012-06/msg00102.html

What do you think?


http://codereview.appspot.com/6294086/diff/1/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (left):

http://codereview.appspot.com/6294086/diff/1/scm/define-grob-properties.scm#oldcode279
scm/define-grob-properties.scm:279: (extra-X-extent ,number-pair? "A
grob is enlarged in
On 2012/06/19 08:53:45, Keith wrote:
> These properties have no effect, so it would be good to remove their
docstrings.
> http://lists.gnu.org/archive/html/lilypond-user/2009-01/msg01154.html

I've opened a separate issue for this:
http://codereview.appspot.com/6294087

http://codereview.appspot.com/6294086/diff/1/scm/define-grob-properties.scm#oldcode964
scm/define-grob-properties.scm:964: (X-extent ,number-pair? "Hard coded
extent in address@hidden")
On 2012/06/19 08:53:45, Keith wrote:
> Maybe "Extent in the X-direction, measured in staff-space units."

Done.

Description:
get rid of minimum-[XY]-extents

they were used long ago for vertical spacing,
now they are deprecated.  Their only use was
with DynamicTextSpanner, but it was more of
an ugly hack than a real solution.

Please review this at http://codereview.appspot.com/6294086/

Affected files:
  M lily/grob.cc
  M python/convertrules.py
  M scm/define-grob-properties.scm
  M scm/define-grobs.scm


Index: lily/grob.cc
diff --git a/lily/grob.cc b/lily/grob.cc
index  
828ae7f07b40687457f8031475f0a42d9992dc67..7cac740e37833c148064b5357474fee7fd5ede30  
100644
--- a/lily/grob.cc
+++ b/lily/grob.cc
@@ -455,14 +455,6 @@ Grob::extent (Grob *refp, Axis a) const
       if (is_number_pair (ext))
         real_ext.unite (ly_scm2interval (ext));

-      SCM min_ext_sym
-        = (a == X_AXIS)
-          ? ly_symbol2scm ("minimum-X-extent")
-          : ly_symbol2scm ("minimum-Y-extent");
-      SCM min_ext = internal_get_property (min_ext_sym);
-      if (is_number_pair (min_ext))
-        real_ext.unite (ly_scm2interval (min_ext));
-
       ((Grob *)this)->dim_cache_[a].extent_ = new Interval (real_ext);
     }

@@ -481,14 +473,6 @@ Grob::pure_height (Grob *refp, int start, int end)
   Interval iv = robust_scm2interval (iv_scm, Interval (0, 0));
   Real offset = pure_relative_y_coordinate (refp, start, end);

-  SCM min_ext = get_property ("minimum-Y-extent");
-
-  /* we don't add minimum-Y-extent if the extent is empty. This solves
-     a problem with Hara-kiri spanners. They would request_suicide and
-     return empty extents, but we would force them here to be large. */
-  if (!iv.is_empty () && is_number_pair (min_ext))
-    iv.unite (ly_scm2interval (min_ext));
-
   if (!iv.is_empty ())
     iv.translate (offset);
   return iv;
@@ -807,8 +791,6 @@ ADD_INTERFACE (Grob,
                "interfaces "
                "layer "
                "meta "
-               "minimum-X-extent "
-               "minimum-Y-extent "
                "outside-staff-horizontal-padding "
                "outside-staff-padding "
                "outside-staff-priority "
Index: python/convertrules.py
diff --git a/python/convertrules.py b/python/convertrules.py
index  
60fb1740fd58826582b775ea6b8280b50e2791b8..3bf0c1b74d912fa433c4114a0377054ff2fe208e  
100644
--- a/python/convertrules.py
+++ b/python/convertrules.py
@@ -3368,6 +3368,12 @@ def conv (str):
         stderr_write (_ ("beamExceptions controls whole-measure beaming.")  
+ "\n")
     return str

address@hidden ((2, 15, 41), r"Remove minimum-[XY]-extent")
+def conv (str):
+    str = re.sub (r"minimum-X-extent", r"X-extent", str)
+    str = re.sub (r"minimum-Y-extent", r"Y-extent", str)
+    return str
+
 # Guidelines to write rules (please keep this at the end of this file)
 #
 # - keep at most one rule per version; if several conversions should be  
done,
Index: scm/define-grob-properties.scm
diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index  
6502dd9d722a4b94d3f5641101e925a86b3d4b17..4f79e1b9b353a5e72cd140646a4c9a9fae570a68  
100644
--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -591,10 +591,6 @@ noteheads.")
 as fraction of note head size.")
      (minimum-space ,ly:dimension? "Minimum distance that the victim
 should move (after padding).")
-     (minimum-X-extent ,number-pair? "Minimum size of an object in
address@hidden, measured in @code{staff-space} units.")
-     (minimum-Y-extent ,number-pair? "Minimum size of an object in
address@hidden, measured in @code{staff-space} units.")


 ;;
@@ -961,7 +957,8 @@ texts.")
 ;;
 ;; x
 ;;
-     (X-extent ,number-pair? "Hard coded extent in address@hidden")
+     (X-extent ,number-pair? "Extent in the address@hidden,
+measured in staff-space units.")
      (X-offset ,number? "The horizontal amount that this object is
 moved relative to its X-parent.")
      (X-positions ,number-pair? "Pair of X staff coordinates of a spanner
@@ -972,7 +969,8 @@ in the form @code{(@var{left} . @var{right})}, where  
both @var{left} and
 ;;
 ;; y
 ;;
-     (Y-extent ,number-pair? "Hard coded extent in address@hidden")
+     (Y-extent ,number-pair? "Extent in the address@hidden,
+measured in staff-space units.")
      (Y-offset ,number? "The vertical amount that this object is moved
 relative to its Y-parent.")

Index: scm/define-grobs.scm
diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm
index  
32f68f5a509a8e4343b53d42d61a39fc3b3ff6e8..9bd19c86430d836b437f52f0ebacc85bd02aa8a6  
100644
--- a/scm/define-grobs.scm
+++ b/scm/define-grobs.scm
@@ -823,8 +823,6 @@
        (left-bound-info . ,ly:line-spanner::calc-left-bound-info-and-text)

        (minimum-length . 2.0)
-       ;; make sure the spanner doesn't get too close to notes
-       (minimum-Y-extent . (-1 . 1))

        (right-bound-info . ,ly:line-spanner::calc-right-bound-info)
        (springs-and-rods . ,ly:spanner::set-spacing-rods)