asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!
http://asciidoctor.org
Apache License 2.0
625 stars 173 forks source link

Introduce public package for AST interfaces #112

Closed mojavelinux closed 10 years ago

mojavelinux commented 10 years ago

The AST interfaces (e.g., Document, Block, Node, etc) should be in a public package, since they are first-class citizens for using Asciidoctor on the JVM.

Taking a page from Eclipse JDT, I propose we name this package "dom".

If there are types that are only public for extensions, then I think they should be in an package named "spi". We may even want to have packages inside of "extensions" too.

mojavelinux commented 10 years ago

Feel free to extend this reorg even more so as to achieve consistency overall in the API.

lordofthejars commented 10 years ago

Completely agree and sorry for not designed as is from start. I think that the best way would be continue as is and not making a big change now (the week of devoxx lot of pull requests, ...), release the preview1 with same packages, but then from preview1 to preview2 change package names.

WDYT?

2013/11/13 Dan Allen notifications@github.com

The AST interfaces (e.g., Document, Block, Node, etc) should be in a public package, since they are first-class citizens for using Asciidoctor on the JVM.

Taking a page from Eclipse JDT, I propose we name this package "dom".

  • org.asciidoctor.dom.AbstractNode
    • org.asciidoctor.dom.Inline
    • org.asciidoctor.dom.AbstractBlock
    • org.asciidoctor.dom.Document
    • org.asciidoctor.dom.Section
    • org.asciidoctor.dom.Block

If there are types that are only public for extensions, then I think they should be in an package named "spi". We may even want to have packages inside of "extensions" too.

  • org.asciidoctor.extensions.Processor
    • org.asciidoctor.extensions.BlockProcessor
    • org.asciidoctor.extensions.internal.ExtensionRegistryExecutor
  • org.asciidoctor.extensions.spi.ExtensionRegistry

— Reply to this email directly or view it on GitHubhttps://github.com/asciidoctor/asciidoctorj/issues/112 .

+----------------------------------------------------------+ Alex Soto Bueno - Computer Engineer www.lordofthejars.com +----------------------------------------------------------+

mojavelinux commented 10 years ago

Agreed. Naturally, these take time to get right. Agile through feedback loop will get us there in the best way.

LightGuard commented 10 years ago

Implementations should stay in an internal package though. We only expose the API interfaces. Otherwise we run the rick of people depending on the implementation classes which may change (but still be API compatable). 

It also allows us to use JS or ruby impls without people caring.  — Sent from Mailbox for iPhone

On Sun, Jan 26, 2014 at 11:18 AM, Alex Soto notifications@github.com wrote:

Closed #112.

Reply to this email directly or view it on GitHub: https://github.com/asciidoctor/asciidoctorj/issues/112

lordofthejars commented 10 years ago

I completely agree with your @LightGuard approach to create like Eclipse interfaces for each element (for example Options, Attributes classes should be interfaces) and create an implementation. But before this second cycle of refactorings I prefer first of all start playing with dynjs and see what we can and what we cannot do, and then be able to purpose an standard API.

For example do you know if with asciidoctorjs we can render files stored locally? Or only we can render strings. Also I need to explore how we would be able to write extensions in Java and run them inside Javascript (in this case I know Dan has to do some work).

LightGuard commented 10 years ago

Well, even if it's only strings we can still feed the impl the string from Java. 

Do we want to change packages for some objects twice? That was my main idea. Though as long as we're still doing these changes in milestone releases people should understand things can change.  — Sent from Mailbox for iPhone

On Sun, Jan 26, 2014 at 11:43 AM, Alex Soto notifications@github.com wrote:

I completely agree with your @LightGuard approach to create like Eclipse interfaces for each element (for example Options, Attributes classes should be interfaces) and create an implementation. But before this second cycle of refactorings I prefer first of all start playing with dynjs and see what we can and what we cannot do, and then be able to purpose an standard API.

For example do you know if with asciidoctorjs we can render files stored locally? Or only we can render strings. Also I need to explore how we would be able to write extensions in Java and run them inside Javascript (in this case I know Dan has to do some work).

Reply to this email directly or view it on GitHub: https://github.com/asciidoctor/asciidoctorj/issues/112#issuecomment-33326010

lordofthejars commented 10 years ago

Yes I have already thought about passing Strings directly from Java, but then the output will not be the same as Asciidoctor Ruby because the output generated by using renderFile method and render method is not the same (some part is stripped as Dan noted in one post). Moreover with Dan we agreed that JS implementation will come with 1.6.0 so I think it will appear for one milestone of next release. Moreover I think we will need to discuss vastly how to do it to make easy fir users to use one or another implementation.

For example how they are going to create options for one env. or the other one?

Options o = new JRubyOptions();
Options o = new  JSOptions();

//or

Options o = options().safeMode()....js();
Options o = options().safeMode()....jruby();

but of course because this class it should be the same for both env. then maybe it should not be splited between interface and implementation, and internally transform to js transparently to users. So each case would be studied separately I guess.

LightGuard commented 10 years ago

Just thinking out loud, but I'd think it would be best to hide the impl from the user, especially for things like the interfaces. We should handle converting to the correct impl, or using the correct impl based on either user selection or presence of ruby. Again, just thinking out loud. — Sent from Mailbox for iPhone

On Sun, Jan 26, 2014 at 11:54 AM, Alex Soto notifications@github.com wrote:

Yes I have already thought about passing Strings directly from Java, but then the output will not be the same as Asciidoctor Ruby because the output generated by using renderFile method and render method is not the same (some part is stripped as Dan noted in one post). Moreover with Dan we agreed that JS implementation will come with 1.6.0 so I think it will appear for one milestone of next release. Moreover I think we will need to discuss vastly how to do it to make easy fir users to use one or another implementation. For example how they are going to create options for one env. or the other one?

Options o = new JRubyOptions();
Options o = new  JSOptions();
//or
Options o = options().safeMode()....js();
Options o = options().safeMode()....jruby();

but of course because this class it should be the same for both env. then maybe it should not be splited between interface and implementation, and internally transform to js transparently to users. So each case would be studied separately I guess.

Reply to this email directly or view it on GitHub: https://github.com/asciidoctor/asciidoctorj/issues/112#issuecomment-33326334

lordofthejars commented 10 years ago

:+1: I am aligned with your thinking :smile: