Move work to compiler #6
Conversation
Previously dicts were parsed and stored in ETS on application startup. This commit introduces modules to get vendors, attributes and values. This saves time during startup of the application. Secondly this opens the possibility to offer a set of macros that will replace the macro with the correct value at compile time instead of the encoding phase at runtime. This should also decrease the amount of times you run into EntryNotFoundError. nimble_parsec is compiled into a module so that users of the library don't have to add nimble_parsec as a dep themselves.
|
This is really a big change, thanks for the work.
|
Come to think of it, with the current default dictionary list we don't parse as many dictionaries as before. Perhaps the default should be all the dictionaries that are included in the main dictionary file? |
|
I'm asking because I had some private dicts, I can just load them at runtime without touching this library, but I don't see how can I do this after this patch . |
|
Would it help if I add an option where you can provide the path to your own dicts? They would need to be available at compile time but they will be compiled into the same module as the other dicts. |
|
this might work, also we should ensure the same dictionaries is loaded by default as before so that it wont break users that assumes this. |
|
Done! :) The only thing that concerns me about this default behaviour is that there are quite a few overlapping values which generates the following warning which can't be avoided: It isn't nice to see this on first compilation. Perhaps this can be reconsidered with a v2? |
| lookup!(name: {vendor, attr, name}) | ||
| end | ||
| for attribute <- attributes do | ||
| attr_fun_name = "attr_#{attribute.name}" |> String.replace("-", "_") |> String.to_atom() |
There was a problem hiding this comment.
nitpick: String.replace("-", "_") should be extract as a mangle_name function
There was a problem hiding this comment.
Agree, in general I think this module needs to be cleaned up and a bit more DRY. However you would have to create the function before calling it at compile time.
If it's okay with you I'd rather merge and then polish in smaller chunks.
Previously dicts were parsed and stored in ETS on application startup. This commit introduces modules and functions to get vendors, attributes and values. This saves time during startup of the application. Secondly this opens the possibility to offer a set of macros that will replace the call with the correct value at compile time instead of the encoding phase at runtime. This should also decrease the amount of times you run into EntryNotFoundError by for example misspelling.
nimble_parsec is compiled into a module so that users of the library don't have to add nimble_parsec as a dep themselves.
The dictionaries are moved to 'dicts' so they will also not be added automatically to a release (I updated mix.exs)
I can see this is a relatively big change. Please don't hesitate to leave any feedback or ask any questions.