New extension API on Java, last step before working

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

New extension API on Java, last step before working

asotobu
Hi,

in https://github.com/asciidoctor/asciidoctorj/tree/new-extension-api you will find current implementation of new extension API of asciidoctor but in asciidoctorJ. There is only one last step before working, and it is that I have some problems integrating Block and Macro Processors. With Pre/Pro/Include/Tree processors works pretty well, but with Block and Macro, next exception is thrown:

org.jruby.exceptions.RaiseException: (ArgumentError) Invalid arguments specified for registering inline macro extension: [:man, "ManpageMacro", {}]
        at RUBY.add_syntax_processor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.ext/lib/asciidoctor/extensions.rb:1030)
        at RUBY.inline_macro(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.ext/lib/asciidoctor/extensions.rb:891)

And inspecting the line where it fails:

def inline_macro *args, &block
      add_syntax_processor :inline_macro, args, &block
end

it seems that three parameters are sent but not the expected type. After fixing this problem, whole new extension API will be available in asciidoctorj too.

I will continue researching anyway.
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator
Alex,

Your work so far looks great!

I've send a pull request to your branch with a partial fix for the problems you were encountering. The main issue is that the order of parameters that you are passing to the registration methods (e.g., block, block_macro) is backwards because the order changed from 0.1.4.

The correct signature is the type or instance of the processor followed by the optional name. You can see examples here:

https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/extensions.rb#L742

The same is true for all "syntax processors".

I also see you are assigning the type "Document" to the parent parameter of the process methods. The parent is not always Document. Sometimes, it's another block, such as a section or open block. Thus, the parent parameter should be "AbstractBlock".

One minor thing I see is that the block name is passed around as a String. Technically, this works because I have symbol conversions on the Ruby side. However, I wonder if we should switch any references to the name from String to Symbol. I'm sort of on the fence about that one because it does introduce some friction when programming. Maybe we can just agree at the integration level what type is to be passed.

Examine the pull request I've sent to see if you have any other questions.

-Dan


On Sun, Feb 16, 2014 at 3:54 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Hi,

in https://github.com/asciidoctor/asciidoctorj/tree/new-extension-api you will find current implementation of new extension API of asciidoctor but in asciidoctorJ. There is only one last step before working, and it is that I have some problems integrating Block and Macro Processors. With Pre/Pro/Include/Tree processors works pretty well, but with Block and Macro, next exception is thrown:

org.jruby.exceptions.RaiseException: (ArgumentError) Invalid arguments specified for registering inline macro extension: [:man, "ManpageMacro", {}]
        at RUBY.add_syntax_processor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.ext/lib/asciidoctor/extensions.rb:1030)
        at RUBY.inline_macro(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.ext/lib/asciidoctor/extensions.rb:891)

And inspecting the line where it fails:

def inline_macro *args, &block
      add_syntax_processor :inline_macro, args, &block
end

it seems that three parameters are sent but not the expected type. After fixing this problem, whole new extension API will be available in asciidoctorj too.

I will continue researching anyway.



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator
In reply to this post by asotobu
I will also add Document as a parameter to the Preprocessor. Look for that update to hit Asciidoctor master shortly.

-Dan


On Sun, Feb 16, 2014 at 6:42 PM, Dan Allen <[hidden email]> wrote:
Alex,

Your work so far looks great!

I've send a pull request to your branch with a partial fix for the problems you were encountering. The main issue is that the order of parameters that you are passing to the registration methods (e.g., block, block_macro) is backwards because the order changed from 0.1.4.

The correct signature is the type or instance of the processor followed by the optional name. You can see examples here:

https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/extensions.rb#L742

The same is true for all "syntax processors".

I also see you are assigning the type "Document" to the parent parameter of the process methods. The parent is not always Document. Sometimes, it's another block, such as a section or open block. Thus, the parent parameter should be "AbstractBlock".

One minor thing I see is that the block name is passed around as a String. Technically, this works because I have symbol conversions on the Ruby side. However, I wonder if we should switch any references to the name from String to Symbol. I'm sort of on the fence about that one because it does introduce some friction when programming. Maybe we can just agree at the integration level what type is to be passed.

Examine the pull request I've sent to see if you have any other questions.

-Dan


On Sun, Feb 16, 2014 at 3:54 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Hi,

in https://github.com/asciidoctor/asciidoctorj/tree/new-extension-api you will find current implementation of new extension API of asciidoctor but in asciidoctorJ. There is only one last step before working, and it is that I have some problems integrating Block and Macro Processors. With Pre/Pro/Include/Tree processors works pretty well, but with Block and Macro, next exception is thrown:

org.jruby.exceptions.RaiseException: (ArgumentError) Invalid arguments specified for registering inline macro extension: [:man, "ManpageMacro", {}]
        at RUBY.add_syntax_processor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.ext/lib/asciidoctor/extensions.rb:1030)
        at RUBY.inline_macro(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.ext/lib/asciidoctor/extensions.rb:891)

And inspecting the line where it fails:

def inline_macro *args, &block
      add_syntax_processor :inline_macro, args, &block
end

it seems that three parameters are sent but not the expected type. After fixing this problem, whole new extension API will be available in asciidoctorj too.

I will continue researching anyway.



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--



--
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
Thank you Dan for your help,

I was looking Ruby code for one hour and I didn't see anything. I need to learn Ruby this 2014 because of my ignorance of Ruby and not familiar with some parts of Asciidoctor, means you are loosing some time in Java part too instead of improving Ruby part.

BTW I will keep an eye to update the Preprocessor method too.

Alex



Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator
Alex,

I think you'll find that Ruby isn't too hard to master enough to understand the depths of Asciidoctor. The key tool, IMO, is "irb". It's the Ruby REPL that you can use to test your understanding as you go along. Of course, using the single-test trick mentioned in the README is also a huge help. It's possible to debug Ruby as well, either using Pry or Rubymine.

I'm going to push out Asciidoctor 1.5.0.preview.2 shortly so that you can integrate against it. Of course, if we find anything we need to change, I'll patch it asap. That's why I'm keeping it as a preview :)

Cheers,

-Dan


On Mon, Feb 17, 2014 at 12:32 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Thank you Dan for your help,

I was looking Ruby code for one hour and I didn't see anything. I need to learn Ruby this 2014 because of my ignorance of Ruby and not familiar with some parts of Asciidoctor, means you are loosing some time in Java part too instead of improving Ruby part.

BTW I will keep an eye to update the Preprocessor method too.

Alex






If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1490.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
I agree with you, but I need to change my mind, I have so hard coded the idea of watching on code the type and the name of the parameters, not only the name, and this is what happened to me in this error, I was watching the parameters and I didn't recall that the types were shifted.

Well anyway, thank you so much.
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
Dan I have implemented the extension API where each kind of extension can be registered as String, Class or instance. For example:

.AsciidoctorModule.java
[source, java]
----
void postprocessor(String postprocessorClassName);
void postprocessor(Class<? extends Postprocessor> postprocessorClassName);
void postprocessor(Postprocessor postprocessor);
----

The string and instance approach works well but the Class one it throws an exception:

org.jruby.exceptions.RaiseException: (ArgumentError) Invalid arguments specified for registering postprocessor extension: [class org.asciidoctor.extension.CustomFooterPostProcessor, {}]
        at RUBY.add_document_processor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/extensions.rb:1036)
        at RUBY.postprocessor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/extensions.rb:653)

I think that the problem is that in extension API another condition is required:

.extensions.rb
[source, ruby]
----
# style 2: specified as class or class name
        if (processor.is_a? ::Class) || ((processor.is_a? ::String) && (processor = Extensions.class_for_name processor))
          unless processor < kind_class || (kind_java_class && processor < kind_java_class)
            raise ::ArgumentError.new %(Invalid type for #{kind_name} extension: #{processor})
          end
          processor_instance = processor.new config
          processor_instance.freeze
          ProcessorExtension.new kind, processor_instance
        # style 3: specified as instance
        elsif (processor.is_a? kind_class) || (kind_java_class && (processor.is_a? kind_java_class))
          processor.update_config config
          processor.freeze
          ProcessorExtension.new kind, processor
        else
          raise ::ArgumentError.new %(Invalid arguments specified for registering #{kind_name} extension: #{args})
        end
----

Is it possible that in style #2 it detects only Ruby classes but not Java classes? And in style #3 cannot enter because it is not an instance.

WDYT?

Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator
Alex,

The issue seems to be that the object that gets passed to Ruby is a real Java class (JavaClass) instead of a Ruby class proxy (RubyClass).

There appear to be two ways to solve this problem.

== Solution #1

Wrap the Java class in a proxy class. To do this, you first need to change the signature in the AsciidoctorModule to:

```java
void postprocessor(RubyClass postprocessorClass);
```

Then you need to wrap the Java class in a proxy in the JavaExtensionRegistry:

```java
this.asciidoctorModule.postprocessor(JavaClass.get(this.rubyRuntime, postprocesorClass).getProxyClass());
```

This solution will work with Asciidoctor 1.5.0.preview.2.

== Solution #2

The second solution is to change the Ruby side to detect a Java class object and convert it to a Ruby class. Above the code you included in your last post, I would need to add:

```ruby
if (defined? ::Java) && (processor.is_a? ::Java::JavaLang::Class)
  processor = processor.ruby_class
end
```

I tried both solutions and both work. What do you think is the best option?

-Dan


On Tue, Feb 18, 2014 at 3:16 PM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Dan I have implemented the extension API where each kind of extension can be registered as String, Class or instance. For example:

.AsciidoctorModule.java
[source, java]
----
void postprocessor(String postprocessorClassName);
void postprocessor(Class<? extends Postprocessor> postprocessorClassName);
void postprocessor(Postprocessor postprocessor);
----

The string and instance approach works well but the Class one it throws an exception:

org.jruby.exceptions.RaiseException: (ArgumentError) Invalid arguments specified for registering postprocessor extension: [class org.asciidoctor.extension.CustomFooterPostProcessor, {}]
        at RUBY.add_document_processor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/extensions.rb:1036)
        at RUBY.postprocessor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/extensions.rb:653)

I think that the problem is that in extension API another condition is required:

.extensions.rb
[source, ruby]
----
# style 2: specified as class or class name
        if (processor.is_a? ::Class) || ((processor.is_a? ::String) && (processor = Extensions.class_for_name processor))
          unless processor < kind_class || (kind_java_class && processor < kind_java_class)
            raise ::ArgumentError.new %(Invalid type for #{kind_name} extension: #{processor})
          end
          processor_instance = processor.new config
          processor_instance.freeze
          ProcessorExtension.new kind, processor_instance
        # style 3: specified as instance
        elsif (processor.is_a? kind_class) || (kind_java_class && (processor.is_a? kind_java_class))
          processor.update_config config
          processor.freeze
          ProcessorExtension.new kind, processor
        else
          raise ::ArgumentError.new %(Invalid arguments specified for registering #{kind_name} extension: #{args})
        end
----

Is it possible that in style #2 it detects only Ruby classes but not Java classes? And in style #3 cannot enter because it is not an instance.

WDYT?




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1507.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
I think the first one is better because we avoid to put some Java stuff inside Ruby part. Moreover a lot of adaptions are done in Java part so this is another one to do.
Thank you so much for your help on extensions part 

El dimecres, 19 febrer de 2014, mojavelinux [via Asciidoctor :: Discussion] <[hidden email]> va escriure:
Alex,

The issue seems to be that the object that gets passed to Ruby is a real Java class (JavaClass) instead of a Ruby class proxy (RubyClass).

There appear to be two ways to solve this problem.

== Solution #1

Wrap the Java class in a proxy class. To do this, you first need to change the signature in the AsciidoctorModule to:

```java
void postprocessor(RubyClass postprocessorClass);
```

Then you need to wrap the Java class in a proxy in the JavaExtensionRegistry:

```java
this.asciidoctorModule.postprocessor(JavaClass.get(this.rubyRuntime, postprocesorClass).getProxyClass());
```

This solution will work with Asciidoctor 1.5.0.preview.2.

== Solution #2

The second solution is to change the Ruby side to detect a Java class object and convert it to a Ruby class. Above the code you included in your last post, I would need to add:

```ruby
if (defined? ::Java) && (processor.is_a? ::Java::JavaLang::Class)
  processor = processor.ruby_class
end
```

I tried both solutions and both work. What do you think is the best option?

-Dan


On Tue, Feb 18, 2014 at 3:16 PM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Dan I have implemented the extension API where each kind of extension can be registered as String, Class or instance. For example:

.AsciidoctorModule.java
[source, java]
----
void postprocessor(String postprocessorClassName);
void postprocessor(Class<? extends Postprocessor> postprocessorClassName);
void postprocessor(Postprocessor postprocessor);
----

The string and instance approach works well but the Class one it throws an exception:

org.jruby.exceptions.RaiseException: (ArgumentError) Invalid arguments specified for registering postprocessor extension: [class org.asciidoctor.extension.CustomFooterPostProcessor, {}]
        at RUBY.add_document_processor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/extensions.rb:1036)
        at RUBY.postprocessor(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/extensions.rb:653)

I think that the problem is that in extension API another condition is required:

.extensions.rb
[source, ruby]
----
# style 2: specified as class or class name
        if (processor.is_a? ::Class) || ((processor.is_a? ::String) && (processor = Extensions.class_for_name processor))
          unless processor < kind_class || (kind_java_class && processor < kind_java_class)
            raise ::ArgumentError.new %(Invalid type for #{kind_name} extension: #{processor})
          end
          processor_instance = processor.new config
          processor_instance.freeze
          ProcessorExtension.new kind, processor_instance
        # style 3: specified as instance
        elsif (processor.is_a? kind_class) || (kind_java_class && (processor.is_a? kind_java_class))
          processor.update_config config
          processor.freeze
          ProcessorExtension.new kind, processor
        else
          raise ::ArgumentError.new %(Invalid arguments specified for registering #{kind_name} extension: #{args})
        end
----

Is it possible that in style #2 it detects only Ruby classes but not Java classes? And in style #3 cannot enter because it is not an instance.

WDYT?




If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1507.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1509.html
To unsubscribe from New extension API on Java, last step before working, click here.
NAML


--
Enviat amb Gmail Mobile
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
BTW it seems an strange "behaviour" of JRuby because there are some types that are bound automatically but in case of Class type you need to manually convert.

Ok I will do it :D We are almost there, this week I will try to release preview2 of asciidoctorj too.
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator

Great!

I agree the necessity for manual conversion in this case seems strange. Perhaps something to note down and ask the JRuby team about when we get the chance.

-Dan

On Feb 19, 2014 12:42 AM, "asotobu [via Asciidoctor :: Discussion]" <[hidden email]> wrote:
BTW it seems an strange "behaviour" of JRuby because there are some types that are bound automatically but in case of Class type you need to manually convert.

Ok I will do it :D We are almost there, this week I will try to release preview2 of asciidoctorj too.


If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1513.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
I have been playing a bit trying to resolve the problem of integration with InlineMacroExtension.

Basically I have seen next new code on Ruby class:

[source, Ruby]
----
 def initialize name, config = {}
      super
      @config[:regexp] ||= (resolve_regexp @name, @config[:format])
    end

    def resolve_regexp name, format
      # TODO memoize these regular expressions!
      if format == :short
        %r(\\?#{name}:\[((?:\\\]|[^\]])*?)\])
      else
        %r(\\?#{name}:(\S+?)\[((?:\\\]|[^\]])*?)\])
      end
    end
----

So as start I have tried to do the same in the test so the config object received in Java class already contains the regexp attribute:
 
[source, java]
----
ByteList pattern = ByteList.create("man" + ":(\\S+?)\\[.*?\\]");
       
options.put(":regexp", RubyRegexp.newRegexp(JRubyRuntimeContext.get(), pattern));
       
ManpageMacro inlineMacroProcessor = new ManpageMacro("man", options);

javaExtensionRegistry.inlineMacro(inlineMacroProcessor);
----

And anyway a received next exception:

org.jruby.exceptions.RaiseException: (TypeError) wrong argument type NilClass (expected Regexp)
        at org.jruby.RubyString.gsub(org/jruby/RubyString.java:3037)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:535)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:534)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:116)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:105)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/block.rb:81)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:583)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:49)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:62)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at org.jruby.RubyArray.map(org/jruby/RubyArray.java:2409)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:828)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:248)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:51)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:809)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1347)
        at RUBY.render_file(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1435)
        at RUBY.render_file(<script>:62)
        at org.jruby.gen.InterfaceImpl1855613620.render_file(org/jruby/gen/InterfaceImpl1855613620.gen:13)



I have some other ideas to try to improve this situation, (now I am thinking that maybe RegExp JRuby class is transformed wrongly in RubyHash method), I will keep you informed about the progress, but if you think I am in the wrong direction of the problem don't hesitate to tell me :)

Thank you so much for your attention.
Alex.
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator

Hey Alex,

I'll make sure to take a look at this today. At the very least I should be able to sort out whether this is a JRuby translation issue or a problem in core.

Cheers,

-Dan

On Feb 23, 2014 3:48 AM, "asotobu [via Asciidoctor :: Discussion]" <[hidden email]> wrote:
I have been playing a bit trying to resolve the problem of integration with InlineMacroExtension.

Basically I have seen next new code on Ruby class:

[source, Ruby]
----
 def initialize name, config = {}
      super
      @config[:regexp] ||= (resolve_regexp @name, @config[:format])
    end

    def resolve_regexp name, format
      # TODO memoize these regular expressions!
      if format == :short
        %r(\\?#{name}:\[((?:\\\]|[^\]])*?)\])
      else
        %r(\\?#{name}:(\S+?)\[((?:\\\]|[^\]])*?)\])
      end
    end
----

So as start I have tried to do the same in the test so the config object received in Java class already contains the regexp attribute:
 
[source, java]
----
ByteList pattern = ByteList.create("man" + ":(\\S+?)\\[.*?\\]");
       
options.put(":regexp", RubyRegexp.newRegexp(JRubyRuntimeContext.get(), pattern));
       
ManpageMacro inlineMacroProcessor = new ManpageMacro("man", options);

javaExtensionRegistry.inlineMacro(inlineMacroProcessor);
----

And anyway a received next exception:

org.jruby.exceptions.RaiseException: (TypeError) wrong argument type NilClass (expected Regexp)
        at org.jruby.RubyString.gsub(org/jruby/RubyString.java:3037)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:535)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:534)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:116)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:105)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/block.rb:81)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:583)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:49)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:62)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at org.jruby.RubyArray.map(org/jruby/RubyArray.java:2409)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:828)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:248)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:51)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:809)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1347)
        at RUBY.render_file(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1435)
        at RUBY.render_file(<script>:62)
        at org.jruby.gen.InterfaceImpl1855613620.render_file(org/jruby/gen/InterfaceImpl1855613620.gen:13)



I have some other ideas to try to improve this situation, (now I am thinking that maybe RegExp JRuby class is transformed wrongly in RubyHash method), I will keep you informed about the progress, but if you think I am in the wrong direction of the problem don't hesitate to tell me :)

Thank you so much for your attention.
Alex.


If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1529.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
Cool i have been trying different things this afternoon and I have found nothing
BTW you Can use asciidoctorj branch. There you Can find the latest version. All extensions except inlinemacro ready to be used.

El diumenge, 23 febrer de 2014, mojavelinux [via Asciidoctor :: Discussion] <[hidden email]> va escriure:

Hey Alex,

I'll make sure to take a look at this today. At the very least I should be able to sort out whether this is a JRuby translation issue or a problem in core.

Cheers,

-Dan

On Feb 23, 2014 3:48 AM, "asotobu [via Asciidoctor :: Discussion]" <[hidden email]> wrote:
I have been playing a bit trying to resolve the problem of integration with InlineMacroExtension.

Basically I have seen next new code on Ruby class:

[source, Ruby]
----
 def initialize name, config = {}
      super
      @config[:regexp] ||= (resolve_regexp @name, @config[:format])
    end

    def resolve_regexp name, format
      # TODO memoize these regular expressions!
      if format == :short
        %r(\\?#{name}:\[((?:\\\]|[^\]])*?)\])
      else
        %r(\\?#{name}:(\S+?)\[((?:\\\]|[^\]])*?)\])
      end
    end
----

So as start I have tried to do the same in the test so the config object received in Java class already contains the regexp attribute:
 
[source, java]
----
ByteList pattern = ByteList.create("man" + ":(\\S+?)\\[.*?\\]");
       
options.put(":regexp", RubyRegexp.newRegexp(JRubyRuntimeContext.get(), pattern));
       
ManpageMacro inlineMacroProcessor = new ManpageMacro("man", options);

javaExtensionRegistry.inlineMacro(inlineMacroProcessor);
----

And anyway a received next exception:

org.jruby.exceptions.RaiseException: (TypeError) wrong argument type NilClass (expected Regexp)
        at org.jruby.RubyString.gsub(org/jruby/RubyString.java:3037)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:535)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:534)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:116)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:105)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/block.rb:81)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:583)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:49)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:62)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at org.jruby.RubyArray.map(org/jruby/RubyArray.java:2409)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:828)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:248)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:51)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:809)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1347)
        at RUBY.render_file(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1435)
        at RUBY.render_file(<script>:62)
        at org.jruby.gen.InterfaceImpl1855613620.render_file(org/jruby/gen/InterfaceImpl1855613620.gen:13)



I have some other ideas to try to improve this situation, (now I am thinking that maybe RegExp JRuby class is transformed wrongly in RubyHash method), I will keep you informed about the progress, but if you think I am in the wrong direction of the problem don't hesitate to tell me :)
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1531.html
To unsubscribe from New extension API on Java, last step before working, click here.
NAML


--
Enviat amb Gmail Mobile
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator
Alex,

I got to the bottom of the issue and sent a pull request with updated.

There were a few issues with the exchange, which primarily revolved around strings not being converted to symbols. I fixed it up so that the tests all pass for the inline macros.

While working on the code, I had the feeling that perhaps we need to think of a cleaner way to pass symbols. Right now there are times when symbol conversion happens transparently and sometimes where you have to be explicit about it. I'd rather make the use of the colon prefix (e.g., ":type") required in all cases where a symbol is expected.

We can still handle the conversion transparently, but we shouldn't be converting strings that aren't prefixed with a colon to a symbol. The reason is that then the user of the API isn't always sure when the colon is needed and when it isn't. Better that we match the expectations of the Ruby API. wdyt?

I also find it strange in JRuby that the symbol wasn't a reserved type, like Symbol. That would make it easy to do:

 options.put(new Symbol("format"), new Symbol("short"));

That would allow us to use generics to enforce where symbols are required. Granted, it is more verbose than:

 options.put(":format", ":short");

though in the second case we can't enforce the presence of the colon. I'm not sure which is the right option. But it's definitely better than the magic that currently happens:

                                 v explicit symbol
 options.put("format", ":short");
                  ^ no indication of symbol

Cheers,

-Dan


On Sun, Feb 23, 2014 at 1:47 PM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote:
Cool i have been trying different things this afternoon and I have found nothing
BTW you Can use asciidoctorj branch. There you Can find the latest version. All extensions except inlinemacro ready to be used.

El diumenge, 23 febrer de 2014, mojavelinux [via Asciidoctor :: Discussion] <[hidden email]> va escriure:

Hey Alex,

I'll make sure to take a look at this today. At the very least I should be able to sort out whether this is a JRuby translation issue or a problem in core.

Cheers,

-Dan

On Feb 23, 2014 3:48 AM, "asotobu [via Asciidoctor :: Discussion]" <[hidden email]> wrote:
I have been playing a bit trying to resolve the problem of integration with InlineMacroExtension.

Basically I have seen next new code on Ruby class:

[source, Ruby]
----
 def initialize name, config = {}
      super
      @config[:regexp] ||= (resolve_regexp @name, @config[:format])
    end

    def resolve_regexp name, format
      # TODO memoize these regular expressions!
      if format == :short
        %r(\\?#{name}:\[((?:\\\]|[^\]])*?)\])
      else
        %r(\\?#{name}:(\S+?)\[((?:\\\]|[^\]])*?)\])
      end
    end
----

So as start I have tried to do the same in the test so the config object received in Java class already contains the regexp attribute:
 
[source, java]
----
ByteList pattern = ByteList.create("man" + ":(\\S+?)\\[.*?\\]");
       
options.put(":regexp", RubyRegexp.newRegexp(JRubyRuntimeContext.get(), pattern));
       
ManpageMacro inlineMacroProcessor = new ManpageMacro("man", options);

javaExtensionRegistry.inlineMacro(inlineMacroProcessor);
----

And anyway a received next exception:

org.jruby.exceptions.RaiseException: (TypeError) wrong argument type NilClass (expected Regexp)
        at org.jruby.RubyString.gsub(org/jruby/RubyString.java:3037)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:535)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.sub_macros(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:534)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:116)
        at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
        at RUBY.apply_subs(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/substitutors.rb:105)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/block.rb:81)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:583)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:49)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:62)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at org.jruby.RubyArray.map(org/jruby/RubyArray.java:2409)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/abstract_block.rb:68)
        at RUBY.content(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:828)
        at RUBY.result(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/html5.rb:248)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/backends/base_template.rb:51)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/renderer.rb:151)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor/document.rb:809)
        at RUBY.render(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1347)
        at RUBY.render_file(/Users/alex/git/asciidoctorj/target/test-classes/gems/asciidoctor-1.5.0.preview.2/lib/asciidoctor.rb:1435)
        at RUBY.render_file(<script>:62)
        at org.jruby.gen.InterfaceImpl1855613620.render_file(org/jruby/gen/InterfaceImpl1855613620.gen:13)



I have some other ideas to try to improve this situation, (now I am thinking that maybe RegExp JRuby class is transformed wrongly in RubyHash method), I will keep you informed about the progress, but if you think I am in the wrong direction of the problem don't hesitate to tell me :)
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1531.html
To unsubscribe from New extension API on Java, last step before working, click here.
NAML


--
Enviat amb Gmail Mobile



If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/New-extension-API-on-Java-last-step-before-working-tp1486p1533.html
To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML



--
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

mojavelinux
Administrator
In reply to this post by asotobu

On Mon, Feb 24, 2014 at 6:46 PM, Dan Allen <[hidden email]> wrote:
I got to the bottom of the issue and sent a pull request with updated.

                                                                                                     ^ code :)
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
In reply to this post by mojavelinux
Totally agree with you Dan. It was my mistaken. When I started with asciidoctorj, I wrongly assume that symbols only would appear as option key, so because I thought it was only in one place to bind the symbols creation transparently. But then the things has been complicated a bit and the result is the one you have seen, becoming a not a coherent API.

So I completely agree that we must standarize the API.
Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
Dan I have been thinking a bit today about the problem with options. In fact we already have the same problem with attributes, some of them can be symbols but others not, and applies to keys and values.

Another problem we have is that for example when user wants to set options the Map containing options can be of kind Map<String, Object> and transform by "magic" to RubyObjects, some RubySymbols others not. But what happens with extensions is that although the Map is defined as Map<String, Object>, the key is a RubyObject (because they are objects that live inside Ruby). This has been fixed in current version using an instance of, but again if some new values are inserted only these new additions should be transformed because the previous ones are already RubyObjects.

So we need something different. One option could be avoid users use Maps. They can only interact with Options and Attributes class, so we can make the required transformations.

Another thing we can do is create something like you suggested:

Map<Symbol, Object> options;

so now at least we can ensure that keys are symbols, but again we still some of the same problems as previous approach:

* User passes a Symbol object as key, but when these objects lives inside Ruby they become RubySymbols. You can see the problem in Processor class createBlock method:


RubyHash convertMapToRubyHashWithSymbols = RubyHashUtil.convertMapToRubyHashWithSymbolsIfNecessary(rubyRuntime,
                options);
createBlock is called in extensions when they want to create a new block:

return createBlock(parent, "pass", Arrays.asList(content), attributes, this.getConfig());

and as you can see getConfig() is a Ruby call.

* With attributes we still have the same problem, we cannot do Map<Symbol, Object> but Map<Object, Object> so we are not forcing user to use Symbols.


Arrived at this point I think that maybe the best solution is avoid users use Map, and implies that public API, extensions, ... only can use Options and Attributes.

Of course Options class will have a generic addOption(String, Object), but we can create a helper class something like:

Symbol.toSymbol("format");

which basically appends the ":" at the start, and we leave the responsibility to user to put a symbol when requires. Then inside addOption(key, value) method we can do something like:

if(key.startsWith(":")) {
toSymbol
} else {
toRubyObject
}
}

And the same for values (well in fact for values is a bit more complex because can be another maps, ...).


Also I have been thinking another radical approach (but personally I don't see the benefits) and it is create some kind of hierarchy types, something like BasicType, IntegerType extends BasicType, SymbolType extends BasicType, ...

So we can do something like:

Map<SymbolType, BasicType>

But I said I don't like because in fact we are mimicking JRuby structure and in fat we are converting one hierarchy to another one.  But maybe could be a solution,

Ant thoughts?


Reply | Threaded
Open this post in threaded view
|

Re: New extension API on Java, last step before working

asotobu
Also I remember now that I started to not forcing users to use the ruby syntax for symbols because I wanted that final users didn't notice that a Ruby gem was being used under the covers. So this is something we must decide if we prefer that final users knows that or not.