Re: Implements DOM-id property for grobs. (issue 5504106)

[mtsolo]

Reviewers: Patrick McCarty, dak, mike_apollinemike.com, janek,


http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc
File lily/grob.cc (right):

http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode177
lily/grob.cc:177: SCM expr = scm_list_3 (ly_symbol2scm ("id"),
On 2012/01/21 18:15:25, janek wrote:
> isn't 'id' a scheme-thingy already?  I mean, in line 174, it is
declared as SCM,
> so why convert it with ly_symbol2scm?

We checked previously for a grob property called "id" that needs to be a
string.  Here, we are creating a stencil.  All stencils are represented
internally by lists where the first element is the name of the stencil
in symbol form.

http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode178
lily/grob.cc:178: id,
On 2012/01/21 18:15:25, janek wrote:
> why store id two times?

The first id is the symbol id.
The second is the variable "id" that contains something else (foobar or
whatever).

http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode179
lily/grob.cc:179: retval.expr ());
On 2012/01/21 18:15:25, janek wrote:
> do i understand correcly that retval stands for "return value" and is
some kind
> of an object?

Yup.  It should always be another stencil.  This is a convention int he
code base.

http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode181
lily/grob.cc:181: retval = Stencil (retval.extent_box (), expr);
On 2012/01/21 18:15:25, janek wrote:
> Do we overwrite the retval that was set earlier (in other if's)?
Why?

This is a way of saying "the retval is equal to the old retval wrapped
in something new".  Here, the new wrapping is an id.  Earlier in
grob.cc, it's a color.

http://codereview.appspot.com/5504106/diff/11013/lily/stencil-interpret.cc
File lily/stencil-interpret.cc (right):

http://codereview.appspot.com/5504106/diff/11013/lily/stencil-interpret.cc#newcode81
lily/stencil-interpret.cc:81: (*func) (func_arg, scm_list_2
(ly_symbol2scm ("start-enclosing-id-node"), id));
On 2012/01/21 18:15:25, janek wrote:
> this line is longer than 80 chars, do we care about it?

I don't think so.

http://codereview.appspot.com/5504106/diff/11013/lily/stencil-interpret.cc#newcode82
lily/stencil-interpret.cc:82: interpret_stencil_expression (scm_caddr
(expr), func, func_arg, o);
On 2012/01/21 18:15:25, janek wrote:
> i understand that we extract actual stencil here and interpret it
again, but
> what these func's do?

Depends what the func is.  It's a way to pass a function as an argument.
 Look for interpret_stencil_expression calls and you'll see what's
passed in & why.

Description:
Implements DOM-id property for grobs.

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

Affected files:
  A input/regression/id.ly
  M lily/grob.cc
  M lily/stencil-interpret.cc
  M scm/define-grob-properties.scm
  M scm/define-stencil-commands.scm
  M scm/output-ps.scm
  M scm/output-svg.scm


Index: input/regression/id.ly
diff --git a/input/regression/id.ly b/input/regression/id.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..10c628f3a8a549c286c94d50e2deaf32660e7fee
--- /dev/null
+++ b/input/regression/id.ly
@@ -0,0 +1,9 @@
+\version "2.15.27"
+
+\header {
+  texidoc = "Shows the id property of a grob being set.  This should have
+no effect in the PS backend.
+"
+}
+
+{ \override NoteHead #'id = #"foo" c }
Index: lily/grob.cc
diff --git a/lily/grob.cc b/lily/grob.cc
index  
6c2eba1710fb15a283f5ecd50249f3ee7f11320f..ed96f1d13a9b1386a4c64912c2d823684b397097  
100644
--- a/lily/grob.cc
+++ b/lily/grob.cc
@@ -170,6 +170,17 @@ Grob::get_print_stencil () const
             = *unsmob_stencil (scm_call_1 (ly_lily_module_constant  
("stencil-whiteout"),
                                            retval.smobbed_copy ()));
         }
+
+      SCM id = get_property ("id");
+      if (scm_is_string (id))
+        {
+          SCM expr = scm_list_3 (ly_symbol2scm ("id"),
+                                 id,
+                                 retval.expr ());
+
+          retval = Stencil (retval.extent_box (), expr);
+        }
+
     }

   return retval;
@@ -784,6 +795,7 @@ ADD_INTERFACE (Grob,
                "cause "
                "color "
                "cross-staff "
+               "id "
                "extra-X-extent "
                "extra-Y-extent "
                "extra-offset "
Index: lily/stencil-interpret.cc
diff --git a/lily/stencil-interpret.cc b/lily/stencil-interpret.cc
index  
1d89e032ba2559051bc8d489fc339eacc7450df2..8214af5dcc8e40a427dcd80212a0931a74bcef6f  
100644
--- a/lily/stencil-interpret.cc
+++ b/lily/stencil-interpret.cc
@@ -74,6 +74,16 @@ interpret_stencil_expression (SCM expr,

           return;
         }
+      else if (head == ly_symbol2scm ("id"))
+        {
+          SCM id = scm_cadr (expr);
+
+          (*func) (func_arg, scm_list_2 (ly_symbol2scm  
("start-enclosing-id-node"), id));
+          interpret_stencil_expression (scm_caddr (expr), func, func_arg,  
o);
+          (*func) (func_arg, scm_list_1 (ly_symbol2scm  
("end-enclosing-id-node")));
+
+          return;
+        }
       else if (head == ly_symbol2scm ("rotate-stencil"))
         {
           SCM args = scm_cadr (expr);
Index: scm/define-grob-properties.scm
diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index  
4e0cc6af8e6899a154a84fb73c5282fef4e6a92d..28443694a686e457542c77af7d9f9db74acf46d7  
100644
--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -483,6 +483,13 @@ left and one to the right of this grob.")
 ;;
 ;; i
 ;;
+     (id ,string? "An id string for the grob.  Depending on the typestting
+backend being used, this id will be assigned to a group containing all of
+the stencils that comprise a given grob.  For example, in the svg backend,
+the string will be assigned to the @code{id} attribute of a group (<g>)
+that encloses the stencils that comprise the grob.  In the Postscript
+backend, as there is no way to group items, the setting of the id property
+will have no effect.")
      (ignore-collision ,boolean? "If set, don't do note collision
 resolution on this @code{NoteColumn}.")
      (implicit ,boolean? "Is this an implicit bass figure?")
Index: scm/define-stencil-commands.scm
diff --git a/scm/define-stencil-commands.scm  
b/scm/define-stencil-commands.scm
index  
d7e930cd7e0415a41b9f8c33c2c59f0b9f7f8672..c8a6b179622d6b071064edb6a3e3afbb5d9bfa9a  
100644
--- a/scm/define-stencil-commands.scm
+++ b/scm/define-stencil-commands.scm
@@ -34,6 +34,7 @@ defined in the output modules (@file{output-*.scm})."
     ellipse
     embedded-ps
     embedded-svg
+    end-enclosing-id-node
     glyph-string
     grob-cause
     named-glyph
@@ -52,6 +53,7 @@ defined in the output modules (@file{output-*.scm})."
     setcolor
     setrotation
     setscale
+    start-enclosing-id-node
     text
     unknown
     url-link
@@ -69,6 +71,7 @@ are used internally in  
@file{lily/@/stencil-interpret.cc}."
     combine-stencil
     delay-stencil-evaluation
     footnote
+    id
     rotate-stencil
     scale-stencil
     translate-stencil
Index: scm/output-ps.scm
diff --git a/scm/output-ps.scm b/scm/output-ps.scm
index  
82fe77d14042a4987e650d3915fa8487db42ce77..0b4405072ea2a6edc370e8ca140a327c59cf4dfe  
100644
--- a/scm/output-ps.scm
+++ b/scm/output-ps.scm
@@ -77,6 +77,12 @@
      "false")
    radius thick))

+(define (start-enclosing-id-node s)
+  "")
+
+(define (end-enclosing-id-node)
+  "")
+
 (define (dashed-line thick on off dx dy phase)
   (ly:format "~4f ~4f ~4f [ ~4f ~4f ] ~4f draw_dashed_line"
    dx
Index: scm/output-svg.scm
diff --git a/scm/output-svg.scm b/scm/output-svg.scm
index  
5d1c09c99d5fa2ddc554072b5f5e38f80c1628f8..27b6118483ccdb05cfcb1fce79e805a037637d2f  
100644
--- a/scm/output-svg.scm
+++ b/scm/output-svg.scm
@@ -70,6 +70,12 @@
   "c = close"
   (format #f "</~S>\n" entity))

+(define (start-enclosing-id-node s)
+  (string-append "<g id=\"" s "\">\n"))
+
+(define (end-enclosing-id-node)
+  "</g>\n")
+
 (define-public (comment s)
   (string-append "<!-- " s " -->\n"))