Please add a way to unregister a specific extension (make extension API symmetric)

[lefou]

When registering a extension class, e.g. via the JavaExtensionRegistry, there is no way to unregister the same extension besides unregistering all. In ExtensionRegistry there is no unregister support at all.

To implement extension support in OSGi via the Whiteboard Pattern (see #297) a way to unregister an extension is needed.

[mojavelinux]

Seems reasonable to add. In the current design in core we store extensions by groups, so the first step would be to allow a single group to be unregistered. If necessary, we could go deeper.

It appears like we need to open an issue in core, right?

[lefou]

@mojavelinux With core do you mean the Ruby project?

[robertpanzer]

I think so. Otherwise I would probably have to modify Asciidoctor::Extensions::Registry.@groups directly.

[mojavelinux]

Correct, "core" means Asciidoctor Ruby. As Robert points out, without a method he needs to go for internal access, which is not ideal. Best to strike at the root first.

[lefou]

Im still looking forward to asciidoctor/asciidoctor#1701 as a prerequisite of this issue, so that we can have proper OSGi support with 1.6.x.

[mojavelinux]

I'm starting to finalize the 1.5.6 release, so I'll make sure this gets in.

[lefou]

asciidoctor/asciidoctor#2265 brings direct support for unregistration of plug-ins.
It would be great, if we could have that functionality also available in the Java API. @robertpanzer, do you see opportunity to work on this? If not, do you mind to give me some hint so that I or others can try to add it?

[mojavelinux]

That's absolutely the intention. We'll want to add an unregisterExtension method to the Asciidoctor class. This complements the existing unregisterAllExtensions method. The new method should accept either a string or a collection of strings. These strings are the extension group names.

See https://github.com/asciidoctor/asciidoctor/blob/a38de38a15a6bde00d82f687a52601894b096d60/lib/asciidoctor/extensions.rb#L1402-L1410

@robertpanzer Asciidoctor core uses symbols for the group names. However, I've added logic to coerce the argument to a symbol. Let me know if you need me to do this or whether I can assume the argument is a symbol.

[robertpanzer]

I can try to get my fingers on it next week.

If anybody wants to have a try before the first thing would be indeed to complement the method unregisterAllExtensions method and add the way into Ruby here:
https://github.com/asciidoctor/asciidoctorj/blob/asciidoctorj-1.6.0/asciidoctorj-core/src/main/resources/org/asciidoctor/internal/asciidoctorclass.rb

It will be interesting to see what names are being used at the moment when registering extensions.
Without looking at the code I cannot tell atm.

[mojavelinux]

That's going to be the trickery part. We need to figure out where and how a group name is assigned to the registry from AsciidoctorJ. In reality, there is not just one registry, but an arbitrary number of registries each stored under a group name. If a group name is not specified, a name is generated. Right now, that name is created in sequence as extgrp${index}, where ${index} is a zero-based sequence. But the Ruby API accepts a name as an argument.

What we could do is have AsciidoctorJ create a group for each extension registered, using the extension class as the group name.

Though I would imagine in the annotation-based configuration, we could add a @Group annotation that would allow extensions to be bundled together in the registry.

[robertpanzer]

Maybe it would make sense then to have AsciidoctorJ generate a name and make the register methods return a registration object on which unregister could be called?

So for example JavaExtensionRegistry.preprocessor() would automatically generate a random name and pass it to the Ruby implementation.

Registration reg = asciidoctor.javaExtensionRegistry().preprocessor(foo); // foo could be a string, class or instance
// ... do whatever
// and then
reg.remove();

In this case it would actually be good that we didn't apply a method chaining approach for registering multiple processors because it would make such an approach impossible.

The same could be done with the RubyExtensionRegistry.

Wdyt?

[robertpanzer]

Downside of this proposal would be though that it would make the API binary incompatible, leaving the build plugins behind again...

[mojavelinux]

As I mentioned in the pull request, it should be possible to supply an explicit group name. It's crucial for organizing groups of extensions.

To avoid breaking binary compatibility (if that's something we decide to pursue), we could accept a group name as another argument to the processor methods. A string would be an explicit group name. A boolean would indicate whether to auto-generate and return a registration object.

Another possible idea, though harder to implement, is to introduce a new chain that hangs off the "group(String)" method. This method would initiate a group. Then, you would register all extensions through that group object, and it could also be used to unregister the group. That would definitely sidestep the whole API compatibility issue...and it would also be very logical.