Prefix convert methods with convert_

[lulalala]

I saw that in asciidoctor's update last year (asciidoctor/asciidoctor#3150), node methods were renamed with convert_ prefix.

I wonder if the same thing should be done here? If so, I can try to do that.

ping @mojavelinux, @slonopotamus

[slonopotamus]

1. Our converters are inheriting from Asciidoctor::Converter instead of Asciidoctor::Converter::Base where changes that you linked were made, so we have our own logic to dispatch convert methods.
2. We claim to support Asciidoctor >= 1.5.3, so we would need some backward-compat code. Asciidoctor::Converter::Base was only added in 2.0.0: asciidoctor/asciidoctor@87ea9a7
3. We have low test coverage, so care is needed when collecting a list of methods to rename.
4. However, I do like that there would be a clear indication which methods are convert-methods.

[mojavelinux]

Marat is correct that both names work. Great care was taken when developing Asciidoctor 2 to not break existing converters like Asciidoctor EPUB3 and Asciidoctor PDF. The convert_ prefix is preferred once the minimum version of Asciidoctor is 2. (At that time, the converter should move to the new inheritance model as well).

I think this should be considered for Asciidoctor EPUB3 2. My plan is to do the same for Asciidoctor PDF.

[slonopotamus]

We can rename methods (and adjust our dispatcher accordingly) now, and only delay reparenting to Asciidoctor::Converter::Base to Asciidoctor EPUB3 2.

[lulalala]

Thanks. I've made the changes while keeping old methods (with deprecation messages).

[slonopotamus]

I do not think we expect someone to call us programmatically, so keeping deprecated methods is not needed. @mojavelinux correct me if I'm wrong.

[mojavelinux]

That's actually pretty hard to answer. I want to say it's not needed, but then again, I have no idea what people are doing downstream with the converter as I haven't really looked.

[slonopotamus]

I believe they shouldn't do anything more than require 'asciidoctor-epub3'. And from there on all usage goes through Asciidoctor core defined by converter API.

@lulalala let's drop that deprecation stuff, okay?

[lulalala]

@slonopotamus my friend expressed interest in writing the spec for conversion methods inContentConverter.

• Would you like such unit tests?
• Would you like such PR to come before this, or would you prefer to have that PR after this is merged?
• Any other classes you also want to have unit tests?

Just want do double check since my friend has never done open source contribution before so it's best that the contribution will be something that matches what you prefer :D

[slonopotamus]

My current plan is:

1. Get some feedback on cannot load such file -- kindlegen on Linux #268
2. Release alpha-12
3. Merge resolves #279 prefix node convert methods with convert_ #280 (your PR), but without deprecation stuff
4. Merge Use imagedir from an image's context during packaging #282

#282 brings in severe risk of breakage, so the more imagedir-related tests can be imagined and added to verify that it doesn't break anything, the better.

If you look at existing tests in asciidoctor-epub3 or asciidoctor-pdf, we do not test much on API/class/method level, but instead test conversion process as a blackbox. This allows to freely modify implementation while maintaining consistent output. You may thing about a converter as a big f(x)->y function that accepts asciidoc document (+ small number of configuration options) as an input and produces epub/mobi as an output. This is actually what users care about.

[slonopotamus]

You may also note that I became asciidoctor-epub3 maintainer not so long time ago (around 20th of January), so do not yet have strong knowledge of various quirks and hacks that are done in HTML output of asciidoctor-epub3. Some of them are outdated and no longer true, some are still valid, some stuff needs additional fixing (like #212). It would also be interesting to investigate possibilities of automated testing of visual rendering. asciidoctor-pdf has it and it's very cool from my point of view. But asciidoctor-epub3 likely can't go that route, we would need to actually run ebook program and capture its rendered picture. Dunno, as I said, it needs investigation. Created #285 for this.