Update the generated code comment and add "_gen" filename postfix

[owenwaller]

As per the "go generate help" documentation the standard form for a comment that indicates generated code is defined by the regex:

^// Code generated .* DO NOT EDIT.$

The comment inserted by the generator has been updated to reflect the recommended form.

In addition the generated go files are now named "*_gen.go" So for example

"0_components_vgen.go" => "0_components_gen.go" "main_wasm.go" => "main_wasm_gen.go"

[bradleypeabody]

Thanks for this. We might need to discuss in more detail. The original concept behind generating main_wasm.go was that it made sense to generate in order to make it quick to start a project, but I also specifically intended it so that if someone wants to edit main_wasm.go they can. If we switch this over to being purely code generated and it gets clobber each time, then it makes to so if there's any wiring that needs to be done in main_wasm.go then it's problematic because you cannot edit it.

I guess if we can be reasonably sure no one will need to edit anything in main_wasm.go, then this change could work, but I'm not convinced yet that it's the case.

As another way of approaching this, maybe we could make it so we output main_wasm_gen.go but only if main_wasm.go doesn't exist? This way someone can rename main_wasm_gen.go to main_wasm.go, remove the top comment and just edit it until their heart's content and vugugen won't interfere. What do you think of that approach?

[powerman]

What do you think of that approach?

Personally I don't like it. It's non-standard and have high chance to be misused (people will forget to remove header, for example, or will rename it to something else and thus won't block future re-generation).

Maybe it's possible to split this file into two parts: one which is unlikely change (and thus will be output into file with "_gen" suffix with standard header and will be re-generated each time) and second which might change and will be output without "_gen" suffix and without header and only if that second file won't exists?

Is there any parts of current file which user may need to modify except rootBuilder := &Root{} line?

[owenwaller]

@bradleypeabody can we agree that renaming "0_components_vgen.go" => "0_components_gen.go" and adding the new style generated comment is fine?

But....I don't think what you are proposing works, and I agree with @powerman.

In my CI pipelines if a file is autogenerated, then it is autogenerated by the pipeline each and every time.

But @powerman has actually pointed out something more fundamental. The line:

rootBuilder := &Root{}

Is the real problem. The RHS uses a user defined and user named struct. But the code generation hard codes the user defined and user named struct as Root.

So as far as I can work out, any current vugu app has to have a struct named Root in it, and that Root struct has to act as the top level struct (akin to the top level window control in a Desktop application). Then any auto-generated code will just work. My guess is that this is how everyone in the community is doing this, because that's what all the examples show.

If this isn't the case, then a developer has to know that they have to create their own main(), and what to do. To be fair this is documented at: https://www.vugu.org/doc/program. But it's not entirely clear what you would have to do if your "root" component was called "Banana" for example. And in this case the autogenerated code will fail, as you also need to know to tell the vugugen tool not to generate a main() in this case which is a additional command line switch.

I think a better approach is to:

a) Pass a parameter that is the name of the root component to vugugen (so in the //go:generate comment??) b) Pass the mount point name (again to vugugen in the //go:generate comment) c) Auto-generate a VuguStartEventLoop(...) function, via a Go template, that takes the mount point and root component name as parameters d) Optionally generate a stub main() function that calls the VuguStartEventLoop() function.

Note: even this approach is limited because you'd have to make sure that the name of the struct of the root component passed on the cmd line to vugugen is indeed the struct name used in the code. Off the top of my head I don't see an easy way to enforce this.

Thoughts?

[owenwaller]

I forgot to say....

@bradleypeabody and we have to remember that folks are very likely using the existing devserver.go file to serve the vugu app (at least in development) and not the new Taskfile based approach that we are proposing.

The current tooling, because it rebuilds everything on each update before serving, has also baked in the main_wasm.go file that is the autogenerated one, which hard codes Root as the top level struct.

Although we are pre v1.0 we need to be careful that we don't break everyone's existing projects without a significant benefit for either the project or the community.

[bradleypeabody]

@owenwaller @powerman

Yeah I think this does point at the root issue:

The line: rootBuilder := &Root{} Is the real problem. The RHS uses a user defined and user named struct. But the code generation hard codes the user defined and user named struct as Root.

I think when we talk about "code generation" there are two entirely different concepts:

• The code generation that vugugen normally does is essentially "compiling" (or "lowering" to use compiler-speak) a .vugu file into the next lower form of code. This process is intended to be entirely run by machine and the user should, ideally, never interact with this step at all (there's an argument for being able to read the output for debugging purposes, but certainly never editing a generated file).
• Completely separate from this, there is a concept of generating files that are intended to be edited by the user. While this may sound odd at first, this actually extremely common. go mod init creates a go.mod file, which is intended to be edited later (either by hand or via other tools). The instructions for creating a new Vue or React project generate all kinds of files which are then intended to be edited later. I realize perhaps this is a bit less common than main_wasm.go, but the concept is the same. I do not think we can control every possible use case that people would need for main_wasm.go, nor should we. Instead the file is intended as a starting point so people can then hack in whatever they need. After, vugu programs are indeed just programs. And in my experience, frameworks which hide away the entrypoint from the developer are typically terrible and overcomplicated. I feel very strongly that we should not prevent users from mess with the entrypoint to the program - custom wiring, various random cases, who knows, but the moment we say "no you cannot touch main()", in my opinion, the whole value of the library drops significantly, because we just took a whole bunch of things people might want to do with it and said no, you can't, and for no real good reason.

So with that said, I do agree that the current workflow can cause confusion, so I'm totally open to moving main_wasm.go generation to a different step. Perhaps we need (and this could be put into the new vgutil tool perhaps), a separate project generation tool - and main_wasm.go can be created as part of that? I made vgrun -new-from-example= earlier for this purpose, but all that does is clone a git repo. So maybe something that explicitly makes the various things that are supposed to be there upon initial project creation - would that be a better way of handling main_wasm.go plus anything else that comes up in this category?

[owenwaller]

So for the minute I have reversed the marking main_wasm_gen.go as autogenerated. See ae341f5. Hopefully this allows the rest of the commit which should be uncontroversial to be merged.

I think we should open up the issue of how we deal with main_wasm.go in a new issue, so we can gain soem community feedback on what they tend to do. Is everyone is agreeable to that?

Note: Pending the merge of the new Taskfile build, see PR #263, if building locally the process has to be tweaked to ensure that any previously generated (and committed *_vgen.go) are removed.

The sequence is:

find ./vgform -type f -name "*_vgen.go" -exec rm {} \;
find ./wasm-test-suite -type f -name "*_vgen.go" -exec rm {} \;
go install ./cmd/vugugen
cd ./vgform && go generate
go test -v ./devutil
go test -v ./distutil/
go test -v ./domrender/
go test -v ./gen
go test -v ./js
go test -v ./staticrender/
go test -v ./simplehttp/
go test -v ./vgform/
go test -v -timeout=35m ./wasm-test-suite/

PR #263 does all of this for you, so the sequence is always correct.

To github workflow hasn't been updated to reflect this new sequence. That is planned to be updated to the new Taskfile approach.

Note: Once PR #263 this is merged we should remove any `*_vgen.go" files from the repo that have been previously committed to the repo.

[bradleypeabody]

@owenwaller Thanks and I agree let's move out the main_wasm discussion to another issue. I'll go ahead and merge this so we can continue with the rest of the build sort out in #263 and figure out what to do with main_wasm separately.