-
Notifications
You must be signed in to change notification settings - Fork 173
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
Add an API method to require a library #253
Comments
You can see where this change will benefit in the build plugins:
There should also be a method that accepts a |
Should we include two require methods? One for Ruby and anther for Java?: asciidoctor.requireRubyLibrary("asiidoctor-diagram");
asciidoctor.requireJavaLibrary(...); |
I don't think we need one for requiring a Java library since that's handled very differently in the JVM. Ruby libraries are always loaded dynamically, as are JavaScript libraries. Thus, I think we can let the underlying implementation figure out how to handle the request. |
Ok, added the following to the api: asciidoctor.requireLibrary("asciidoctor-diagram");
asciidoctor.requireLibrary("one", "two", "three");
asciidoctor.requireLibraries(libraryCollection); Thoughts? |
I have a doubt about the use of the Ruby context. I've seen that internally it is stored in a static variable inside |
Nope, it's the other way around. The JRubyRuntimeContext stores a link to the runtime being used by the Asciidoctor instance in the current thread. If anything, that pointer would be wrong. But the Ruby instance inside the Asciidoctor instance is isolated. |
That's not to suggest that the way we handle the Ruby instance is absolutely correct. Just that if there is an issue, it's a broader discussion than this change here. I actually think the way we lookup the Ruby runtime should require passing the Asciidoctor instance in rather than storing it as a single thread local. In other words, there is a two way mapping between the Ruby instance and the Asciidoctor instance so you always end up with the right one. But again, that's a separate discussion from this change because here we are working with the bound instance. |
I see, the new methods work on the context associated in the instance and do not use any of the static methods. |
I don't close this issue until we have a green bar on CI in Windows environment. |
Green bar! |
Add a simple API method, preferably to the Asciidoctor interface, for requiring a library in the underlying runtime. For example:
The method require the library in whatever way is appropriate for the environment in use using the runtime associated with the Asciidoctor instance. This ensures the operation is declarative and not coupled to the underlying runtime.
The text was updated successfully, but these errors were encountered: