Merge pull request sparklemotion#2814 from etiennebarrie/typeddata-xm…

…l-element-content Migrate XML::ElementContent to the TypedData API
ellaklara · Mar 6, 2023 · 34bac70 · 34bac70
2 parents 1f3daf6 + 7b5f470
commit 34bac70
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 37 deletions.
diff --git a/ext/nokogiri/xml_element_content.c b/ext/nokogiri/xml_element_content.c
@@ -2,111 +2,113 @@
 
 VALUE cNokogiriXmlElementContent;
 
+const rb_data_type_t element_content_data_type = {
+  .wrap_struct_name = "Nokogiri::XML::ElementContent",
+};
+
 /*
  * call-seq:
- *  name
+ *   name → String
  *
- * Get the require element +name+
+ * [Returns] The content element's +name+
  */
 static VALUE
 get_name(VALUE self)
 {
   xmlElementContentPtr elem;
-  Data_Get_Struct(self, xmlElementContent, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
 
   if (!elem->name) { return Qnil; }
   return NOKOGIRI_STR_NEW2(elem->name);
 }
 
 /*
  * call-seq:
- *  type
+ *   type → Integer
  *
- * Get the element content +type+.  Possible values are PCDATA, ELEMENT, SEQ,
- * or OR.
+ * [Returns] The content element's +type+. Possible values are +PCDATA+, +ELEMENT+, +SEQ+, or +OR+.
  */
 static VALUE
 get_type(VALUE self)
 {
   xmlElementContentPtr elem;
-  Data_Get_Struct(self, xmlElementContent, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
 
   return INT2NUM(elem->type);
 }
 
 /*
- * call-seq:
- *  c1
- *
  * Get the first child.
  */
 static VALUE
 get_c1(VALUE self)
 {
   xmlElementContentPtr elem;
-  Data_Get_Struct(self, xmlElementContent, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
 
   if (!elem->c1) { return Qnil; }
   return noko_xml_element_content_wrap(rb_iv_get(self, "@document"), elem->c1);
 }
 
 /*
- * call-seq:
- *  c2
- *
- * Get the first child.
+ * Get the second child.
  */
 static VALUE
 get_c2(VALUE self)
 {
   xmlElementContentPtr elem;
-  Data_Get_Struct(self, xmlElementContent, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
 
   if (!elem->c2) { return Qnil; }
   return noko_xml_element_content_wrap(rb_iv_get(self, "@document"), elem->c2);
 }
 
 /*
  * call-seq:
- *  occur
+ *   occur → Integer
  *
- * Get the element content +occur+ flag.  Possible values are ONCE, OPT, MULT
- * or PLUS.
+ * [Returns] The content element's +occur+ flag. Possible values are +ONCE+, +OPT+, +MULT+ or +PLUS+.
  */
 static VALUE
 get_occur(VALUE self)
 {
   xmlElementContentPtr elem;
-  Data_Get_Struct(self, xmlElementContent, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
 
   return INT2NUM(elem->ocur);
 }
 
 /*
  * call-seq:
- *  prefix
+ *   prefix → String
  *
- * Get the element content namespace +prefix+.
+ * [Returns] The content element's namespace +prefix+.
  */
 static VALUE
 get_prefix(VALUE self)
 {
   xmlElementContentPtr elem;
-  Data_Get_Struct(self, xmlElementContent, elem);
+  TypedData_Get_Struct(self, xmlElementContent, &element_content_data_type, elem);
 
   if (!elem->prefix) { return Qnil; }
 
   return NOKOGIRI_STR_NEW2(elem->prefix);
 }
 
+/*
+ *  create a Nokogiri::XML::ElementContent object around an +element+.
+ */
 VALUE
-noko_xml_element_content_wrap(VALUE doc, xmlElementContentPtr element)
+noko_xml_element_content_wrap(VALUE rb_document, xmlElementContentPtr c_element_content)
 {
-  VALUE elem = Data_Wrap_Struct(cNokogiriXmlElementContent, 0, 0, element);
-
-  /* Setting the document is necessary so that this does not get GC'd until */
-  /* the document is GC'd */
-  rb_iv_set(elem, "@document", doc);
+  VALUE elem = TypedData_Wrap_Struct(
+    cNokogiriXmlElementContent,
+    &element_content_data_type,
+    c_element_content
+  );
+
+  /* keep a handle on the document for GC marking */
+  rb_iv_set(elem, "@document", rb_document);
 
   return elem;
 }

diff --git a/ext/nokogiri/xml_element_decl.c b/ext/nokogiri/xml_element_decl.c
@@ -6,7 +6,7 @@ static ID id_document;
 
 /*
  * call-seq:
- *  element_type
+ *   element_type → Integer
  *
  * The element_type
  */
@@ -20,9 +20,9 @@ element_type(VALUE self)
 
 /*
  * call-seq:
- *  content
+ *   content → Nokogiri::XML::ElementContent
  *
- * The allowed content for this ElementDecl
+ * [Returns] The root of this element declaration's content tree.
  */
 static VALUE
 content(VALUE self)
@@ -40,9 +40,9 @@ content(VALUE self)
 
 /*
  * call-seq:
- *  prefix
+ *   prefix → String
  *
- * The namespace prefix for this ElementDecl
+ * [Returns] The namespace +prefix+ for this element declaration.
  */
 static VALUE
 prefix(VALUE self)

diff --git a/lib/nokogiri/xml/element_content.rb b/lib/nokogiri/xml/element_content.rb
@@ -11,8 +11,8 @@ module XML
     #   ]>
     #   </root>
     #
-    # ElementContent represents the tree inside the <!ELEMENT> tag shown above
-    # that lists the possible content for the div1 tag.
+    # ElementContent represents the binary tree inside the <!ELEMENT> tag shown above that lists the
+    # possible content for the div1 tag.
     class ElementContent
       # Possible definitions of type
       PCDATA  = 1

diff --git a/test/xml/test_element_content.rb b/test/xml/test_element_content.rb
@@ -17,7 +17,7 @@ def setup
           <root/>
         eoxml
         @elements = @xml.internal_subset.children.find_all do |x|
-          x.type == 15
+          x.type == Nokogiri::XML::Node::ELEMENT_DECL
         end
         @tree = @elements[1].content
       end