Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Hi,
I am writing this post to show the problem we have with Ruby Symbols and AsciidoctorJ. Let me start with Options. At the begin of the AsciidoctorJ and Asciidoctor all Options were of the form of a Map where key was always a symbol (:safe) and an object which could be of any type but because of Asciidoctor it was never a ruby symbol, so it could be a Map, integer, boolean, string, ... Because of this fact, I decided to hide this details from AsciidoctorJ, so they effectively do something like map.put("safe", 10), and AsciidoctorJ internally transform this string to RubySymbol. At this time no problem. The problem comes when because of change both key and values can be symbols. At this point I cannot transform value to symbol blindly. So what I decided is to make users add the ":" in value, so when I parse the map, what I inspect as well is if the value starts with ":", and if it is the case, I create a substring(1) and I transform it to RubySymbol. And this problem has been scaled everywhere, at the point that Options can receive a map (attributes) which key can be symbol or not and value can be a symbol or not, so in this case I need to inspect all entries (key and value) to see if they start with ":" or not, and acts accordantly. And today I have found the last problem. Some operations from Asciidoctor Ruby returns a map which it should be reachable from caller. And now the problem key and value can be of type RubySymbol, so I need to scan all entries, see if it is instance of RubySymbol and if it is the case return a string value. But what kind of string, one starting with ":" or without?. One may said that why not to rise RubySymbol to caller? Well the answer is that one day we will have a AsciidoctorJ using Asciidoctor.js so I would like to use the same interface and "ideally" switchable (although I think it is not gonna be possible). WDYT? Any ideas? |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
Thanks for writing this up, Alex! Alex has described the workaround we're using, and it's limitations, of transparently converting Strings to Symbol objects to simplify the AsciidoctorJ API. However, what I think is missed is why having Symbols in the API adds friction for the consumer. To restate what Alex said above, the Asciidoctor API requires Symbols in many places, most often as keys to option hashes. If we didn't do anything special in the AsciidoctorJ API, the consumer would be required to create Symbol objects to pass to the API. In general, I don't have a problem with requiring Symbol objects in the API. They are a type and the API requires the use of that type. The challenge (i.e., burden) has to do with how Symbols are implemented. Challenge #1:: The RubySymbol object requires access to the Ruby runtime object to create an instance. In code, that looks like: RubySymbol symbol = RubySymbol.newSymbol(rubyRuntime, key); The caller of the AsciidoctorJ API doesn't have the Ruby runtime object because that's hidden inside of the implementation. In Ruby, a Symbol is defined in much the same way as a String (:symbol vs 'symbol'). Thus, it should only be necessary to create a type like RubySymbol (or Symbol) to pass: Symbol symbol = new Symbol(key); Treating Symbol as just another wrapper type gets to the heart of what I'm asking for in JRuby. Challenge #2:: The use of RubySymbol couples the AsciidoctorJ API to the Ruby implementation. Although it doesn't hurt to import the JRuby API and use RubySymbol just as a wrapper type, it should be possible to have a JRuby-agnostic way of defining a Symbol. Perhaps this is just a matter of us introducing a Symbol class in AsciidoctorJ and transparently converting it to RubySymbol in the JRuby integration code. This would be a similar approach to what we are doing today with a String that starts with a colon...except it's would be more explicit. In summary, I think that challenge #1 is the truly invasive problem. It requires that we couple all parts of the code to the Ruby runtime object (passing it around or exposing it via a ThreadLocal) for the sole purpose of creating Symbols...a type that is supposed to be extremely simple to use. I think the situation has been made to be over-complicated by how JRuby handles the the symbol data type. One possible approach is to stop using symbols in Asciidoctor. Ruby 2.2 makes this much more attractive since String key access in a Hash nearly matches that of symbols. I find this change to erode the readability of the API, though, not to mention slower performance on Ruby < 2.2, so I'm reluctant to do it. Cheers, -Dan On Tue, Dec 16, 2014 at 2:08 PM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote: Hi, ... [show rest of quote] Dan Allen | http://google.com/profiles/dan.j.allen |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
In reply to this post by asotobu
After giving this some more though, I think the right approach is for AsciidoctorJ to introduce the Symbol wrapper type, then convert it internally to a RubySymbol at the interface between Java and Ruby. Although this does require deep cloning of hashes and other objects, it reduces the friction while ensuring clean data is passed across the interface. Of course, we can still convert Strings that start with colon for keys, but for values I think we should require our Symbol wrapper type. It would look something like: ```ruby import org.asciidoctor.Symbol; ... Map<Symbol, Object> options = new HashMap<Symbol, Object>(); options.put(new SymboI("type"), new Symbol("link")); ``` It's a bit more to type, but it's consistent with what the Ruby API requires. We could always provide some helpers in AsciidoctorJ to create these types of maps. Cheers, -Dan On Tue, Dec 16, 2014 at 4:53 PM, Dan Allen <[hidden email]> wrote:
... [show rest of quote] Dan Allen | http://google.com/profiles/dan.j.allen |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Yes I like that approach, only one problem that I will need to make some research. Let me put this example:
AbstractNode interface has a method getOptions that returns a Map<Object, Object>. This is the interface to the real Ruby object, so Map should be of type Object because it can be any Ruby object like RubySymbol, RubyString, ... The problem is with the class that is used by users, in this case AbstractNodeImpl. In this class we are responsible of casting the Map<Object, Object> to Map<Symbol, Object>. The problem is that because AbstractNodeImpl implements AbstractNode, then the user will still receive a Map<Object, Object> although we know that it should be a Map<Symbol, Object>. So for what I see is that we can cast the Map but still returning a Map of Object, Object and it will be the caller responsible to cast the Map to required type. Has it sense? |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
Alex, Is it possible to intercept certain methods? In this case, we can catch the object coming out of Ruby and coerce it. If not, then I think returning a generic Object is the right approach and it still requires some coaxing by the user. I'm more concerned about making it easy to pass data into Ruby...by allowing a lightweight Symbol object as a placeholder for a Ruby symbol. It's pretty clear we'll need to iterate to get this right. Perhaps it's something we want to try to tackle as part of the refactorings in 1.6.0. You are definitely right that we'll need to study the various scenarios. Cheers, -Dan On Wed, Dec 17, 2014 at 1:23 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote: Yes I like that approach, only one problem that I will need to make some research. Let me put this example: ... [show rest of quote] Dan Allen | http://google.com/profiles/dan.j.allen |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Yes ok I am going to open an issue about this change, and creates the first (but not best) solution, so we can see on action. With code I am sure it will be easier to see what to change.
|
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
Indeed! And even better with tests. -Dan On Wed, Dec 17, 2014 at 4:10 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote: Yes ok I am going to open an issue about this change, and creates the first (but not best) solution, so we can see on action. With code I am sure it will be easier to see what to change. Dan Allen | http://google.com/profiles/dan.j.allen |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Hi, today I have found a bug on JRuby which affects us in our classes design.
Currently we have an interface which will be proxied to Ruby instance. For example: public interface AbstractNode { boolean isRole(); } As you may notice Abstract node maps to role? method in Ruby class which returns a boolean. No problem. Then we have the Impl version, which it is used to adapt basically maps to not be a RubySymbol but a String. public abstract class AbstractNodeImpl implements AbstractNode { protected Ruby runtime; protected AbstractNode abstractNode; public AbstractNodeImpl(AbstractNode abstractNode, Ruby ruby) { this.runtime = ruby; this.abstractNode = abstractNode; } @Override public boolean isRole() { return this.abstractNode.isRole(); } } The problem is that because of a bug of JRuby boolean instances are transformed to RubyStrings. So an expcetion is thrown at runtime. I have tried to programmatically cast the Boolean to RubyString (because in fact it is a RubyString) and then parse the String to boolean without luck. So my idea will be the next one: AbstractNode interface is going to be called IAbstractNode. And AbstractNodeImpl is going to be splitted into AbstractNode interface and RubyAbstractNode class which implements previous interface. Then IAbstractNode will return in out case Object isRole(), and inside RubyAbstractNode I will transform the RubyString to Boolean. With this approach we are going to have more freedom to transform what Ruby returns/requires to Java classes. WDYT? |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
Alex, I'll take a deeper look tonight (or this weekend). However, my initial reaction is that we should not be hacking around bugs in JRuby. We should just not implement that method until we get it fixed in JRuby. I really want to keep the AsciidoctorJ code as hack free as possible :) Also, to answer your question about which methods to implement...I wouldn't implement any method that doesn't appear in the Asciidoctor backends repository (https://github.com/asciidoctor/asciidoctor-backends/tree/master/slim/html5). If the method is used there, then I would say the answer is "yes". If it is not used there, then the answer should be "hold" until we find a case for it. The reason I say that is because not all the methods in the AbstractNode and descendants is intended to be public. In the future, I'm going to start marking methods as public or private to be more clear about what's part of the API. Cheers, -Dan On Fri, Dec 19, 2014 at 11:58 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote: Hi, today I have found a bug on JRuby which affects us in our classes design. ... [show rest of quote] Dan Allen | http://google.com/profiles/dan.j.allen |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Cool you Can get the converters branch and try yourself, I talked with @headius and told me it probably be a bug on JRuby El ds., 20 de des., 2014 a les 2.56 mojavelinux [via Asciidoctor :: Discussion] <[hidden email]> va escriure:
... [show rest of quote]
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:
|
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
After some research I think that the problem is more deep that what I mentioned previously. See next test:
@Test public void should_be_able_to_get_roles() { Document document = asciidoctor.load(ROLE, new HashMap<String, Object>()); AbstractBlock abstractBlock = document.blocks().get(0); assertThat(abstractBlock.getRole(), is("famous")); assertThat(abstractBlock.hasRole("famous"), is(true)); assertThat(abstractBlock.isRole(), is(true)); assertThat(abstractBlock.getRoles(), contains("famous")); } hasRole method returns a boolean and JRuby transforms correctly to boolean. But the test fails when it is executing the isRole (role? in Ruby) in this cases JRuby returns a RubyString instead of a boolean like in has method. So my research now will be in this direction to see what's happening with the is method. |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
Alex, I think I've solved it. I agree that the problem is affecting many methods, but it can be attributed to a single point of failure. JRuby is not able to work out the method name mapping itself between the Java interface and the Ruby class. For example, it is trying to map "isAttr" to "is_attr" instead of "attr?". The error about the conversion of a string to a boolean is actually misleading. The real problem is that it's resolving the wrong method. We can help JRuby find the methods by using method aliases, as I have done in this pull request: If we can find another way to feed JRuby this information, I'm happy to explore other approaches. This approach at least gets the job done. I'd like to maintain these aliases in AsciidoctorJ since we don't want to pollute Asciidoctor core to workaround limitations in JRuby. -Dan On Wed, Dec 24, 2014 at 3:11 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote: After some research I think that the problem is more deep that what I mentioned previously. See next test: ... [show rest of quote] Dan Allen | http://google.com/profiles/dan.j.allen |
Loading... |
Reply to author |
Edit post |
Move post |
Delete this post |
Delete this post and replies |
Change post date |
Print post |
Permalink |
Raw mail |
Administrator
|
In reply to this post by asotobu
On Wed, Dec 24, 2014 at 3:11 AM, asotobu [via Asciidoctor :: Discussion] <[hidden email]> wrote: JRuby returns a RubyString instead of a boolean like in has method I think this is because it's actually resolving the wrong method...a method that does return a string, not a boolean. |
Free forum by Nabble | Edit this page |