Skip to content
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

proposal: encoding/xml: support marshal self-closing tag in short-form mode #59710

Open
ECUST-XX opened this issue Apr 19, 2023 · 9 comments
Open
Labels
Milestone

Comments

@ECUST-XX
Copy link

Goals

Add an option, to support marshal a self-closing tag xml struct in short-form mode which has empty content.

In xml standard, a self-closing tag is permitted.
https://www.w3.org/TR/xml/#sec-starttags

Definitions

func ExampleMarshalShortForm() {
	type Address struct {
		City, State string
	}
	type Person struct {
		XMLName   xml.Name `xml:"person"`
		Id        int      `xml:"id,attr"`
		FirstName string   `xml:"name>first"`
		LastName  string   `xml:"name>last"`
		Age       int      `xml:"age"`
		Height    float32  `xml:"height,omitempty"`
		Married   bool
		Address
		Comment       string   `xml:",comment"`
		FavoriteMusic []string `xml:"favorite_music"`
		LuckyNumber   int      `xml:"lucky_number"`
	}

	v := &Person{Id: 13, FirstName: "John", LastName: ""}
	v.Comment = " Need more details. "
	v.Address = Address{"Hanga Roa", ""}
	v.FavoriteMusic = []string{"Made in Heaven", ""}

	output, err := xml.MarshalIndentShortForm(v, "  ", "    ")
	if err != nil {
		fmt.Printf("error: %v\n", err)
	}

	os.Stdout.Write(output)
	// Output:
	//   <person id="13">
	//       <name>
	//           <first>John</first>
	//           <last />
	//       </name>
	//       <age>0</age>
	//       <Married>false</Married>
	//       <City>Hanga Roa</City>
	//       <State />
	//       <!-- Need more details. -->
	//       <favorite_music>Made in Heaven</favorite_music>
	//       <favorite_music />
	//       <lucky_number>0</lucky_number>
	//   </person>
}

Related questions

#21399

@gopherbot gopherbot added this to the Proposal milestone Apr 19, 2023
@ianlancetaylor
Copy link
Contributor

Just to be clear, I think you are proposing a new function MarshalIndentShortForm. That will be like MarshalIndent, but a tag with no contents will use a self-closing tag. Is that about right?

@ECUST-XX
Copy link
Author

Just to be clear, I think you are proposing a new function MarshalIndentShortForm. That will be like MarshalIndent, but a tag with no contents will use a self-closing tag. Is that about right?

Yes. That's right.

@adonovan
Copy link
Member

adonovan commented May 31, 2023

What is the cost of using a self-closing tag wherever it is allowed?

@tnypxl
Copy link

tnypxl commented Oct 6, 2023

@adonovan Cost in terms of what? Can you elaborate?

@adonovan
Copy link
Member

adonovan commented Oct 6, 2023

tnypxl: Cost in terms of what? Can you elaborate?

I meant all kinds of costs. Obviously any change has the potential to cause spurious failures in existing overly-specific tests, but are there more substantial reasons why not to simply enable it always? The output would be more compact, which seems like a good thing.

@tnypxl
Copy link

tnypxl commented Oct 6, 2023

@adonovan Thanks for the clarification. I do agree with your last point. I am running into this issue while researching ways to build a XML config generator.

Without having looked into the current std lib implemtantion, I imagine an always-on approach would break some tests that expect a closing tag from empty elements. I'd be interested in a struct field tag option (e.g, closeempty or selfclose) that lets me decide on a field by field basis. But I imagine for large complex XML trees, that isn't ideal either.

@ydnar
Copy link

ydnar commented Oct 24, 2023

We have a small change that implements the selfclosing option to the xml struct tag here:

nbio/xml#10

Happy to turn this into a formal proposal + CL if folks are interested.

@msurmeli
Copy link

Having a field based struct tag would allow us to decide whether to use self closing tags per field. Personally I would prefer to see your approach as proposal. @ydnar

@ianlancetaylor
Copy link
Contributor

See #69273 for an alternate proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

7 participants