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

so many files #82

Open
awalterschulze opened this issue Mar 18, 2015 · 8 comments
Open

so many files #82

awalterschulze opened this issue Mar 18, 2015 · 8 comments

Comments

@awalterschulze
Copy link

Hi

I really like this tool.
I even wrote my own walk typewriter, that would generate the following method.
This is really useful for Abstract Syntax Trees (ast).

func (this *MyType) Walk(v interface {
    Visit(v interface{}) interface{}
}) {
    v = v.Visit(this).(interface {
        Visit(v interface{}) interface{}
    })
    if this.MyField != nil {
        this.MyField.Walk(v)
    }
}

It is a little ugly, because of the embedded interface.
What you would actually want is:

type Visitor interface {
  Walk(v interface{}) Visitor
}

func (this *MyType) Walk(v Visitor) {
        v := v.Visit(this)
        if  this.MyField != nil {
        this.MyField.Walk(v)
    }
}

But then everyone would have to import this type.
But lets not digress.

I now find that I just generated about 20 files each containing only this one method.
Would it be possible to have all gen code in one file or one file per typewriter or a command line option to do so?

I don't mind doing the work.
Would you be interested in such a pull request?

@clipperhouse
Copy link
Owner

It’s designed to generate one file per-type-per-typewriter. I think that’s a decent way to do it. How many types are you working with here?

@awalterschulze
Copy link
Author

I was testing it on an ast for a query language so about twenty types.

We use a LOT of go and have MANY other packages with MANY other types.
I suspect we could also generate much more of the code than we currently do.
I think using golang/types is a great way to do that.
And your tool which you built on top of that is almost exactly what I want.
I only need to generate less files, because it becomes hard to spot the
files with actual code between all the generated files.

I think it should be pretty easy to support both one file mode and one file
per-type-per-typewriter mode or are you set on only supporting the one file
per-type-per-typewriter mode?

On 18 March 2015 at 20:59, Matt Sherman notifications@github.com wrote:

It’s designed to generate one file per-type-per-typewriter. I think that’s
a decent way to do it. How many types are you working with here?


Reply to this email directly or view it on GitHub
#82 (comment).

@clipperhouse
Copy link
Owner

I’m not set on it. It’s a good default, but very much a matter of taste. Some config might be a solution. Sounds like you want to mark up all your types, each of which gets its own strongly-typed walker?

typewriter.Type does embed a types.Type – which in turn allows you to inspect fields if you assert it to a *types.Struct. Not sure if that helps…we discuss it here.

@awalterschulze
Copy link
Author

I do not have a problem with typewriter.Type it was easy to get it work and
generate code for each field.
That is why I like the your framework, it is easy to use :)

My only problem is the number of files that are generated.
I would like to know if you are open to a new command line flag
-onefile=true or something like that?
This will then generate only one file per package containing all the
generated code.

On 20 March 2015 at 00:42, Matt Sherman notifications@github.com wrote:

I’m not set on it. It’s a good default, but very much a matter of taste.
Some config might be a solution. Sounds like you want to mark up all your
types, each of which gets its own strongly-typed walker?

typewriter.Type does embed a types.Type – which in turn allows you to
inspect fields if you assert it to a *types.Struct. Not sure if that
helps…we discuss it here
clipperhouse/typewriter#5.


Reply to this email directly or view it on GitHub
#82 (comment).

@clipperhouse
Copy link
Owner

Sure, will think about it. I’m sure that as others work on typewriters, they’ll want some control over that.

@awalterschulze
Copy link
Author

any thoughts?

@francoishill
Copy link
Contributor

The config sounds fairly simple to implement. Why don't you do it @awalterschulze and send a PR?

@awalterschulze
Copy link
Author

If only I had the time :(
I am a little swamped at the moment.

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

No branches or pull requests

3 participants