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

added named struct extraction flag #21

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Conversation

periode
Copy link

@periode periode commented May 26, 2022

Hello!

Here's my suggestion for a flag allowing non-nested structs, discussed in #14 . The diff is a bit noisy, but essentially it takes in a slice of names in the flag, then the structwriter check if the node that is about to be written as an anonymous struct is contained in this slice, and just writes the capitalized version of the name instead.

Let me know what you think!

@miku
Copy link
Owner

miku commented Jun 11, 2022

Thanks for the PR and sorry for the long response time - I'll look at it shortly.

@YaroslavPodorvanov
Copy link
Contributor

YaroslavPodorvanov commented Aug 23, 2022

I think better create flag with name "inline" that by default will be true like now;

Like in this solution JSON-to-Go it is called "Inline type definitions";

@miku you agree with "inline" flag?

@periode
Copy link
Author

periode commented Jan 2, 2024

Hi!

Sorry for the (very long!) delay,

@YaroslavPodorvanov 's suggestion makes sense, happy to change the PR to implement it.

That being said, the current PR allows for the extraction of just specific structs. If we change it to a boolean flag, that means that it either inlines all structs, or outlines (?) all structs, with no in-between or granular control.

I could imagine having both flags existing side by side, e.g.:

  • if inlineStructs is true and replaceStructs is empty: default behaviour, everything inlined
  • if inlineStructs is true and replaceStructs is not empty: extracts the structs specified in the replaceStructs
  • if inlineStructs is false and replaceStructs is empty: extracts all structs
  • if inlineStructs is false and replaceStructs is not empty: extracts only the struct

But maybe I'm making it more complicated than it should be, so perhaps we should go for the boolean flag.

Any thoughts?

@YaroslavPodorvanov
Copy link
Contributor

@periode make sense with tests

@miku
Copy link
Owner

miku commented Jan 2, 2024

Thanks for looking into this. With the -r (and the existing -t) flag it would be possible to do the following now:

$ curl -s https://raw.githubusercontent.com/miku/zek/master/fixtures/e.xml | ./zek -e -t channel
// Channel was generated 2024-01-02 21:48:18 by tir on reka.
type Channel struct {
        XMLName       xml.Name `xml:"channel"`
        Text          string   `xml:",chardata"`
        Title         string   `xml:"title"`         // ESS New Releases (Display...
        Link          string   `xml:"link"`          // http://tinyurl.com/ESSNew...
        Description   string   `xml:"description"`   // New releases from the Ear...
        LastBuildDate string   `xml:"lastBuildDate"` // Mon, 27 Nov 2017 00:06:35...
        Item          []struct {
                Text        string `xml:",chardata"`
                ...
                Readme        string   `xml:"readme"`        // readme | https://geoscan....
                PPIid         string   `xml:"PPIid"`         // 34532, 35096, 35438, 2563...
        } `xml:"item"`
} 

$ curl -s https://raw.githubusercontent.com/miku/zek/master/fixtures/e.xml | ./zek -e -r Channel
// Rss was generated 2024-01-02 21:47:53 by tir on reka.
type Rss struct {
        XMLName xml.Name `xml:"rss"`
        Text    string   `xml:",chardata"`
        Rdf     string   `xml:"rdf,attr"`
        Dc      string   `xml:"dc,attr"`
        Geoscan string   `xml:"geoscan,attr"`
        Media   string   `xml:"media,attr"`
        Gml     string   `xml:"gml,attr"`
        Taxo    string   `xml:"taxo,attr"`
        Georss  string   `xml:"georss,attr"`
        Content string   `xml:"content,attr"`
        Geo     string   `xml:"geo,attr"`
        Version string   `xml:"version,attr"`
        Channel Channel  `xml:"channel"`
} 

But maybe I'm making it more complicated than it should be, so perhaps we should go for the boolean flag.

I believe there are two things you want to do: 1) read XML only, 2) read and write XML.

If you only read XML, then inline is nicer to read in code as it's reducing jumps and somewhat reveals the XML tree in the struct itself (I like the compact output from zek more than some of the other generators I tried).

If you want to read and write XML, you probably need all structs expanded anyway, as you need to build the XML yourself.

Therefore, I'd favor a single boolean flag to expand all nested structs.

@periode
Copy link
Author

periode commented Jan 3, 2024

@miku yes, my use case is to manipulate the structs from the XML, so read/write.

I guess I was trying to have the best of both worlds by only extracting the structs I need to process, and keep the rest for readability, but for now the boolean flag to expand nested structs instead of inlining them should do.

It would be something like this:

cat "<a><b>Berlin</b></a>" | ./zek -I
// A was as generated 2024-01-03 18:47:20 by pierre on archpierre.
type A struct {
  XMLName xml.Name `xml:"a"`
  Text string `xml:",chardata"`
  B B `xml:"b"`
}

type B struct {
  Text string `xml:",chardata"` // Berlin
}

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

Successfully merging this pull request may close these issues.

3 participants