Closed ctaggart closed 8 years ago
Debugging is working! So is the vendor folder without any workarounds.
@matthieudelaro, this is ready to go! Feel free to squash merge this to be one commit.
Very nice, thanks for those new features! Sorry I'm AFK today, I'll look at the code tomorrow.
TL;DR Thanks a lot for your modifications, using the debugger in vscode helps! Issues in your code reveal designs issues of Nut, with respect to overloading parameters in macros. => the design of Nut has got to evolve. This is not a small change. I don't have a good solution yet.
Long version
I tried the code and found one minor issue (which I fixed in branch ctaggart-nut-code: nut --exec
was broken because no project was specified upon creation of the MacroExec in exec.go
The reasons why I didn't merge in master:
nut --exec
creates an instance of MacroBase, which doesn't correspond to the version of the project in use (with version 5, it should be MacroV5). While it seems ok right now, it means that in the future, if MacroV42 implements some default behavior, then those won't be taken into account, because MacroBase will be used instead.The two first points above could be fixed with work-around (copy-paste methods from ProjectV5 to MacroV5, and implement a Project.createMacro()), but it seems to me that it reveals a design issue of Nut, so I think it deserves more attention than simple quick fix. There must be several solutions, and here is a draft I can think of right now:
Since Macro and Project both hold configuration settings, let's delegate them to a new interface Configuration, and a ConfigurationV* class. Then MacroV* and ProjectV** implement Configuration:
Project becomes a Configuration + list of Macro. Macro becomes a Configuration + list of actions. Interface Configuration.getParent() returns a Configuration. Project.getParent() returns a Project (which implements Configuration), and Macro.getParent() returns a Macro.
Go methods are statically linked, and we can't define methods for an interface (cf README section in projectbase.go). So we pack all project.go files in a package called ProjectPkg (e.g), where we define functions taking a Configuration as argument, that deal with overloading and merging of parameters (such as func getOverloadedParameters(conf Configuration) {fetch values from parent, ...}
)
This draft idea has got issues:
Other idea: Macro and Project become a same class Configuration except that, upon parsing, Macro checks that it doesn't have any field "macros" or "based_on", and doesn't require the definition of a "syntax_version".
It would be nice to have this new design allow macros to overload other macros. And to allow future implementation of variables (not environment variables, but configuration variables)
Thanks for the review! Regarding merging values from Project into Macro, merging can be the default design choice, but I chose not to. Given your first reaction, it is probably the right choice to merge the values. May be there can be a flag to turn off merging such as if merge: false
. I'm not exactly sure how I broke nut --exec
yet. I probably won't get a chance to review today. For the most part, I was trying to replace Project references with Macro references. If that isn't sufficient, adding a Project.createMacro()
sounds like a good plan.
nut --exec
was broken because no project was specified upon creation of the MacroExec in exec.go. But don't worry about this one, I fixed it already.
Do you need anything from me? I was planning to let you take it from here.
I've started a blog post about the golang-vscode docker image I made, but I'd like to present it when:
I'll work on this issue at the end of the week.
The problem right now about Windows is that I don't have the configuration neither for Docker Toolbox, nor for Docker for Windows (not enough internal space on my MBA, failed emulating inside a VM, ...).
What I did is that I upgraded a dependency, so that at least now Nut compiles for Windows. But I can't debug to make sure that it works properly. Can you? I hope it works with Docker for Windows once some filepath are handled properly (turn \ into /).
Yeah, I may be able to test on Windows if you point me in the right direction. It will be slightly sad to reinstall Toolbox, but I will be if they don't get volumes working soon with Docker for Windows.
Nice! Let's move to issue Add support for Windows #4 for this matter then, where I added extra explanations about stuff to test, and directions to fix.
I found a solution that seems to be good. I went through most of the implementation, so it should be available soon.
I just pushed in master: ac215764089d411f814476a2b52188868d3d9566. This fixes Nut's architecture to integrate the modifications that you offered in syntax V6:
See details of the commit for the list of other modifications.
The way projects and macros are handled changes a lot, so I'll update the README in the nut/config
package, or in the wiki.
@ctaggart
I've started a blog post about the golang-vscode docker image I made, but I'd like to present it when:
- This or something similar has been merged in.
- volume mounts work on Docker for Windows
- Nut support for Windows has landed in master. Any idea on the timeframe for this?
Now that Nut supports Docker Toolbox on OSX, there must be only small changes to make to fix nut on Windows. Maybe is it a matter of changing the paths from /c/path
to ///c/path
, or such. I added extra error logs to debug.
This is work in progress.Project settings like docker_image and security_opts. This fixes #10 & fixes #11.If you run
./nut code
now, it will launch Visual Studio Code for editing this project. :-)I need to get debugging working by fixing the GOPATH either on the image or in the launch actions.