-
-
Notifications
You must be signed in to change notification settings - Fork 160
✨ xmlgen: Maintain definition order #1929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -436,18 +436,8 @@ pub struct Interface<'a> { | |
| #[serde(rename = "@name", borrow)] | ||
| name: InterfaceName<'a>, | ||
|
|
||
| #[serde(rename = "method", default)] | ||
| methods: Vec<Method<'a>>, | ||
| #[serde(rename = "property", default)] | ||
| properties: Vec<Property<'a>>, | ||
| #[serde(rename = "signal", default)] | ||
| signals: Vec<Signal<'a>>, | ||
| #[serde(rename = "annotation", default)] | ||
| annotations: Vec<Annotation>, | ||
| #[serde(skip)] | ||
| docstring: Option<String>, | ||
| #[serde(skip)] | ||
| telepathy_types: Vec<telepathy::TypeDef>, | ||
| #[serde(default)] | ||
|
Comment on lines
438
to
+439
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 ChatGPT GPT-6 Astra Pro on behalf of zeenix: [P2] Preserve the Serde mapping for interface members. The retained Please map this collection to the XML child elements (e.g. |
||
| children: Vec<Child<'a>>, | ||
| } | ||
|
|
||
| impl<'a> Interface<'a> { | ||
|
|
@@ -457,23 +447,47 @@ impl<'a> Interface<'a> { | |
| } | ||
|
|
||
| /// Returns the interface methods. | ||
| pub fn methods(&self) -> &[Method<'a>] { | ||
| &self.methods | ||
| pub fn methods(&self) -> impl Iterator<Item = &Method<'a>> { | ||
| self.children.iter().filter_map(|child| { | ||
| if let Child::Method(m) = child { | ||
| Some(m) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Returns the interface signals. | ||
| pub fn signals(&self) -> &[Signal<'a>] { | ||
| &self.signals | ||
| pub fn signals(&self) -> impl Iterator<Item = &Signal<'a>> { | ||
| self.children.iter().filter_map(|child| { | ||
| if let Child::Signal(s) = child { | ||
| Some(s) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Returns the interface properties. | ||
| pub fn properties(&self) -> &[Property<'_>] { | ||
| &self.properties | ||
| pub fn properties(&self) -> impl Iterator<Item = &Property<'a>> { | ||
| self.children.iter().filter_map(|child| { | ||
| if let Child::Property(p) = child { | ||
| Some(p) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Return the associated annotations. | ||
| pub fn annotations(&self) -> &[Annotation] { | ||
| &self.annotations | ||
| pub fn annotations(&self) -> impl Iterator<Item = &Annotation> { | ||
| self.children.iter().filter_map(|child| { | ||
| if let Child::Annotation(a) = child { | ||
| Some(a) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| /// Return the content of the Telepathy `tp:docstring` extension element, if any. | ||
|
|
@@ -482,40 +496,110 @@ impl<'a> Interface<'a> { | |
| /// surrounding whitespace trimmed. Note that docstrings are only captured when parsing; | ||
| /// the writer does not emit them. | ||
| pub fn docstring(&self) -> Option<&str> { | ||
| self.docstring.as_deref() | ||
| self.children | ||
| .iter() | ||
| .filter_map(|child| { | ||
| if let Child::Docstring(s) = child { | ||
| Some(s.as_ref()) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .next() | ||
| } | ||
|
|
||
| /// Return the Telepathy type definitions on this interface. | ||
| pub fn telepathy_types(&self) -> &[telepathy::TypeDef] { | ||
| &self.telepathy_types | ||
| pub fn telepathy_types(&self) -> impl Iterator<Item = &telepathy::TypeDef> { | ||
| self.children.iter().filter_map(|child| { | ||
| if let Child::TelepathyTypeDef(t) = child { | ||
| Some(t) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| fn write_xml<W: Write>(&self, w: &mut W) -> std::io::Result<()> { | ||
| write!(w, "<interface name=\"{}\"", escape(self.name.as_str()))?; | ||
| if self.methods.is_empty() | ||
| && self.properties.is_empty() | ||
| && self.signals.is_empty() | ||
| && self.annotations.is_empty() | ||
| { | ||
| if !self.children.iter().any(|child| { | ||
| matches!( | ||
| child, | ||
| Child::Method(_) | Child::Signal(_) | Child::Property(_) | Child::Annotation(_) | ||
| ) | ||
| }) { | ||
| return write!(w, "/>"); | ||
| } | ||
| write!(w, ">")?; | ||
| for method in &self.methods { | ||
| method.write_xml(w)?; | ||
| } | ||
| for property in &self.properties { | ||
| property.write_xml(w)?; | ||
| } | ||
| for signal in &self.signals { | ||
| signal.write_xml(w)?; | ||
| } | ||
| for annotation in &self.annotations { | ||
| annotation.write_xml(w)?; | ||
| for child in &self.children { | ||
| child.write_xml(w)?; | ||
| } | ||
| write!(w, "</interface>") | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] | ||
| pub enum Child<'a> { | ||
| #[serde(borrow)] | ||
| Method(Method<'a>), | ||
| #[serde(borrow)] | ||
| Property(Property<'a>), | ||
| #[serde(borrow)] | ||
| Signal(Signal<'a>), | ||
| Annotation(Annotation), | ||
| Docstring(String), | ||
| #[serde(skip)] | ||
| TelepathyTypeDef(telepathy::TypeDef), | ||
|
Comment on lines
+549
to
+551
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These 2 are not really children so a bit strange to have them here. Besides we don't write neither of them so what's the usecase of having them here? Having them aslo means that our check of "if children present" becomes more inefficient than it would otherwise be.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 ChatGPT GPT-6 Astra Pro on behalf of zeenix: [P2] This also introduces a Serde serialization failure. Keeping these definitions outside the serializable children, as suggested above, avoids this regression; otherwise the serializer needs to filter them explicitly. Please cover this with a direct Serde test— |
||
| } | ||
|
|
||
| impl<'a> From<Method<'a>> for Child<'a> { | ||
| fn from(method: Method<'a>) -> Self { | ||
| Self::Method(method) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> From<Property<'a>> for Child<'a> { | ||
| fn from(property: Property<'a>) -> Self { | ||
| Self::Property(property) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> From<Signal<'a>> for Child<'a> { | ||
| fn from(signal: Signal<'a>) -> Self { | ||
| Self::Signal(signal) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> From<Annotation> for Child<'a> { | ||
| fn from(annotation: Annotation) -> Self { | ||
| Self::Annotation(annotation) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> From<String> for Child<'a> { | ||
| fn from(docstring: String) -> Self { | ||
| Self::Docstring(docstring) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> From<telepathy::TypeDef> for Child<'a> { | ||
| fn from(type_def: telepathy::TypeDef) -> Self { | ||
| Self::TelepathyTypeDef(type_def) | ||
| } | ||
| } | ||
|
|
||
| impl<'a> Child<'a> { | ||
| fn write_xml<W: Write>(&'a self, w: &mut W) -> std::io::Result<()> { | ||
| match self { | ||
| Self::Method(m) => m.write_xml(w), | ||
| Self::Property(p) => p.write_xml(w), | ||
| Self::Signal(s) => s.write_xml(w), | ||
| Self::Annotation(a) => a.write_xml(w), | ||
| Self::Docstring(_) => Ok(()), | ||
| Self::TelepathyTypeDef(_) => Ok(()), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// An introspection tree node (typically the root of the XML document). | ||
| #[derive(Debug, Deserialize, Serialize, Clone, PartialEq)] | ||
| pub struct Node<'a> { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -249,40 +249,37 @@ fn interface<'i>( | |
| self_closing: bool, | ||
| ) -> PResult<Interface<'static>> { | ||
| let name = attrs.name(|n| InterfaceName::try_from(n).map_err(Error::Zbus))?; | ||
| let mut methods = Vec::new(); | ||
| let mut properties = Vec::new(); | ||
| let mut signals = Vec::new(); | ||
| let mut annotations = Vec::new(); | ||
| let mut docstring = None; | ||
| let mut telepathy_types = Vec::new(); | ||
| let mut interface_children = Vec::<crate::Child<'static>>::new(); | ||
| children( | ||
| input, | ||
| tag, | ||
| self_closing, | ||
| |input, child, attrs, sc| match child { | ||
| "method" => { | ||
| methods.push(method(input, child, attrs, sc)?); | ||
| interface_children.push(method(input, child, attrs, sc)?.into()); | ||
| Ok(true) | ||
| } | ||
| "property" => { | ||
| properties.push(property(input, child, attrs, sc)?); | ||
| interface_children.push(property(input, child, attrs, sc)?.into()); | ||
| Ok(true) | ||
| } | ||
| "signal" => { | ||
| signals.push(signal(input, child, attrs, sc)?); | ||
| interface_children.push(signal(input, child, attrs, sc)?.into()); | ||
| Ok(true) | ||
| } | ||
| "annotation" => { | ||
| annotations.push(annotation(input, child, attrs, sc)?); | ||
| interface_children.push(annotation(input, child, attrs, sc)?.into()); | ||
| Ok(true) | ||
| } | ||
| other if is_docstring(other) => { | ||
| docstring = capture_docstring(input, other, sc)?.or(docstring.take()); | ||
| if let Some(docstring) = capture_docstring(input, other, sc)? { | ||
| interface_children.push(docstring.into()); | ||
| } | ||
| Ok(true) | ||
| } | ||
| other => { | ||
| if let Some(def) = telepathy_type_def(input, other, &attrs, sc)? { | ||
| telepathy_types.push(def); | ||
| interface_children.push(def.into()); | ||
| } | ||
| Ok(true) | ||
| } | ||
|
|
@@ -291,12 +288,7 @@ fn interface<'i>( | |
|
|
||
| Ok(Interface { | ||
| name, | ||
| methods, | ||
| properties, | ||
| signals, | ||
| annotations, | ||
| docstring, | ||
| telepathy_types, | ||
| children: interface_children, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can be simpler if we just name
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 ChatGPT GPT-6 Astra Pro on behalf of zeenix: Renaming the local also requires qualifying the |
||
| }) | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.