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

Clean up ThingMLCompiler and Context interfaces #244

Open
jakhog opened this issue Oct 1, 2018 · 1 comment
Open

Clean up ThingMLCompiler and Context interfaces #244

jakhog opened this issue Oct 1, 2018 · 1 comment
Assignees

Comments

@jakhog
Copy link

jakhog commented Oct 1, 2018

IMO, the ThingMLCompiler and Context classes have turned into a bit of a mess, with references to directories or other things being set all over the place. I think it is time to have another look at what those classes contain, and try to move some of their features to arguments in the methods where they are actually needed.

To provide some concrete examples of what is strange:

  • SetInput/Output directory. I don't really know where the InputDirectory is used, and I think the OutputDirectory should be moved as an argument to either the compile-method, or where the generated code is written to actual files.
  • Plugins/Tools are set by the registry when a new compiler is asked for. But those will not propagate when .clone() is called. Maybe it's better to ask the registry for them where it is actually used.
  • Logging, I think there is at least three used ways of writing log output currently. That should definitely not be the case...

If you have experienced any other strangeness, feel free to post them here so we can fix it in a single sweep.

@jakhog jakhog self-assigned this Oct 1, 2018
@brice-morin
Copy link
Collaborator

Not to mention Debug, which should be redone as a ThingML tool. Debug can be quite useful, but the way it has been done turned out quite messy.

The InputDirectory may be used indirectly by some annotation e.g. to include some files like this ../lib/my.file so that we can use relative paths. Though I am not completely sure, maybe we can the path of the thingml element another way. So we need to double check that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants