Import `log` package by default into `*_gen.go`

[miyamo2]

Hello, @bradleypeabody @owenwaller

This PR is based on Issues #181 *_gen.go files generation will be changed in this PR as follows.

package main

import "fmt"
import "reflect"
import "github.com/vugu/vjson"
import "github.com/vugu/vugu"
import js "github.com/vugu/vugu/js"
+ import "log"

/*
omit
*/

// 'fix' unused imports
var _ fmt.Stringer
var _ reflect.Type
var _ vjson.RawMessage
var _ js.Value
+ var _ log.Logger

For example, vgform/*_gen.go changes as follows gist.

commands I executed

go run ./cmd/vugugen -r ./vgform

output https://gist.github.com/miyamo2/5cc2f8b6f81f274188cf260c64653ae5

Not related to this PR, but I modified the following file to pass CI(golangci-lint).

• wasm-test-suite/docker/wasm-test-suite-srv.go

Thank you.

[owenwaller]

Hi @miyamo2

Thanks for this. Issue #181 seems to fallen off mine and @bradleypeabody radar, but it is the sort of thing we need to fix.

In principle I don't have a problem with the change to gen/parser-go.go as that seems like a sensible approach.

But, the second change to wasm-test-suite/docker/wasm-test-suite-srv.go is already on the master branch as far as I can tell.

See:

https://github.com/vugu/vugu/blob/1782dc212655d99ff1ffe9147402e62317ba1944/wasm-test-suite/docker/wasm-test-suite-srv.go#L151

Do you want to try syncing with the master branch and see if that removes the need for the change to wasm-test-suite/docker/wasm-test-suite-srv.go?

@bradleypeabody are you ok importing log directly, as this does seem like a common case, Or do you still prefer running the _gen.go files through goimports? I have seen goimports pick up the wrong package before now though.

Thanks

Owen

[bradleypeabody]

Yeah I guess it's fine to add this log import change on the basis that we already provide the same sort of convenience for fmt and js packages. I'm a little leery of using up more import names by default in exchange for a minor convenience - and I would encourage putting more code in separate .go files rather than in <script type='application/x-go'> tags. But I don't really have a compelling answer for why it's fine to do this with fmt and not log. So yeah I think if we limit this PR to just the changes in parser-go.go, it's fine to merge.

(And the goimports thing is probably a separate topic, since it would provide good convenience but at the cost of requiring an additional tool and could also have interesting side effects of possibly selecting the wrong package as mentioned.)

[miyamo2]

@owenwaller @bradleypeabody

But, the second change to wasm-test-suite/docker/wasm-test-suite-srv.go is already on the master branch as far as I can tell.

It was my mistake, it seems it was an unnecessary change, so I restored them. Thank you.

[owenwaller]

@miyamo2 thank you.

@bradleypeabody do you want o merge this one in this? And agreed with the goimports comment. It's managed to pick the wrong log package for me once today already.

About <script type='application/x-go'>...

Personally (and this is only a personal opinion!) I would remove the ability to use <script type='application/x-go'> if I possibly could and force all of the Go code into separate *.go files. This is one of the things that really threw me a curve ball when I first started trying to do things with vugu. Did the Go code live in the .vugu file or not? And if it did how the heck did it all get compiled. As soon as I stopped that, and put everything in a *.go file named after the component things became easier to understand. So I'd remove these script tags for that reason, but I don't have a js background so this may be normal for anyone who has.

But as the use of the <script type='application/x-go'> tags is all over the documentation and the tests, I can't see removing these being possible anymore. I also don't want to break anyone's existing vugu projects.

What I think we should do is strrongly discourage their use (mark as depreciated, or emit a warning?), update the documentation and all of the vugu code so that vugu itself doesn't do it that way. Ideally for any v1.0 release.

Owen

[bradleypeabody]

Cool. This one is merged. And @owenwaller I created this ticket https://github.com/vugu/vugu/issues/271 to follow up on the question of what to do with script tags.