cosmos72 / gomacro

Interactive Go interpreter and debugger with REPL, Eval, generics and Lisp-like macros
Mozilla Public License 2.0
2.18k stars 94 forks source link

go:generate gomacro -genimport #49

Closed glycerine closed 5 years ago

glycerine commented 5 years ago

Having just been bitten by stale dependencies, I wonder if gomacro could add a flag to do this part of package import:

import _i "import/path/for/your/application"

under command from a go generate run. In other words, if there was a flag (call it -genimport for discussion) to the gomacro binary that caused import _i output to be generated for the current working directory, then a

go:generate gomacro -genimport

line could be added to a file in the package we wish to keep up to date. Upon every build I always run go generate to keep the other parts up to date, and this would result in the x_package.go file also being kept up to date at the same time.

Would that make sense? I didn't see such a flag already but maybe there is already a way? Thanks for considering it.

cosmos72 commented 5 years ago

Yes, it makes sense - it would be a useful addition. I don't promise any release date, but I will try to add it

glycerine commented 5 years ago

now that I know it makes sense, I can probably do a pull request if you point me at where the import _i functionality is implemented.

cosmos72 commented 5 years ago

The implementation is in base/genimport/gen.go, it starts with the writeImportFile() function.

Since it is an unexported function, you will have to write an exported wrapper function.

Check the existing callers for how to set-up the parameters: import _i my/pkg/path corresponds to the parameter mode ImportMode = ImInception, but the tricky part is populating the gpkg *types.Package parameter: you need to call the Import(name, path) method of Interp.Comp.CompGlobals.Importer

glycerine commented 5 years ago

cool. Thanks @cosmos72. I'll give it a try.

cosmos72 commented 5 years ago

Looking at it more in detail, the function createImportFile() in base/genimport/importer.go may be an even better fit - it calls writeImportFile() then actually writes the file.

glycerine commented 5 years ago

I think #50 is ready to go. I thought about adding source imports too. However upon reflection that seems to be almost an orthogonal issue. Using source instead of binary packages is slower but would avoid the chicken-and-egg problem when a library depends on it's own x_package.go to be present to build. Nonetheless, bootstrapping usually isn't that hard. I concluded, therefore, that adding imports via source (in which case the package need not even compile) is best considered a separate issue. I'll open a new, separate one if you think it merits consideration.

cosmos72 commented 5 years ago

You pull request looks good, thanks! :)

The only change I'd suggest is to keep processing any remaining command-line args after -g PATH instead of returning immediately - it makes -g behavior consistent with other options.

glycerine commented 5 years ago

The only change I'd suggest is to keep processing any remaining command-line args after -g PATH instead of returning immediately - it makes -g behavior consistent with other options.

good point. I'll make that change.

cosmos72 commented 5 years ago

This raises another minor, aesthetic point: after -g, should PATH be optional or mandatory?

Making it mandatory forces to write gomacro -g . to refer to the current package (and maybe also needs explicit code that handles .), but on the good side it removes ambiguity if -g is not the last option.

glycerine commented 5 years ago

Mmm. Was justing considering that exact point. I like the -g . idea. The other thing would be to say that the path can be completely optional only if the -g is the last option given.

glycerine commented 5 years ago

Now I remember. The other reason to exit right away was that otherwise I ended up in the gomacro REPL. Heh.

Maybe we're doing too much future planning... consider it "You aren't going to need it" and leave until it really does need to change? The current pr is pretty minimal and difficult to misuse. Update: I'll add the "." special handling though, for sure.

glycerine commented 5 years ago

Ok. Github is hanging on any access to my fork. They might be having storage issues. I even tried deleting and re-creating it (so now the PR looks like it is from "unknown repo"), but I still can't seem to access my fork of gomacro. So I sent you email with the two updated files.

Anyway. It is rebased against the current master, so you should be able to copy those in and see just my diffs with a git diff.

cosmos72 commented 5 years ago

Thanks, I received the email. Good to know that in worst case I can apply your PR manually.

About starting a REPL after executing -g PATH: the default behavior is to start a REPL only if no option "do something" is present. Such default behavior can be overridden by -i, which means "always start a REPL as last thing".

There is an internal flag, used for example by -e EXPR, to implement this mechanism. I will set it also for -g PATH, don't worry.

glycerine commented 5 years ago

Added PR from a different account. So #51 is current; should be same as the files I sent.

I found the repl flag. Much saner now. :)