Asciidoctor :: Discussion

AsciidoctorJ Screenshot Extension

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

5 messages Options

sclassen

AsciidoctorJ Screenshot Extension

Hi Dan

Sorry for starting a new thread. Somehow the web UI at discuss.asciidoctor.org cannot follow the old thread and starts a new topic for every reply :(
I hope this one will work better...

Thanks for pointing out the typo in the repo name.
I spent some time and implemented you suggestions except for the naming of the processors (I would like to give the names a second thought).

Here is what I have done:
* change screenshot from a block processor to a block macro processor
* support :literal blocks in the browse macro processor
* add positional attributes for 'dimension' in browse macro processor
* add positional attributes for 'name' and 'dimension' in screenshot processor
* change frame/dimension names to lower case

Do you think it would be possible to transfer the repo to the official asciidoctor github organization?
I am hoping to get more visibility. Additionally I would be able to publish the extension in the asciidoctor maven repo on bintray.

Cheers
Stephan

sclassen

Re: AsciidoctorJ Screenshot Extension

OK, I gave the naming a second though.

I still prefer the macro to be called "screenshot" and not capture.
Reason for this is that all the other extensions I found also use nouns to describe what is included into the documentations (i.e. image::, link::, issue::, include::).

For the browsing block I currently tend to call it "geb" because it contains GEB code. The reason is similar. I try to come up with a name describing what it is.

Feedback is more than welcomed :)

mojavelinux

Re: AsciidoctorJ Screenshot Extension

Administrator

Great work Stephan!

I just tried the example and I'm quite impressed by the result! This is powerful!

I have one question. How do I use PhantomJS or Chrome instead of Firefox? It wasn't clear from the README how to change browsers.

Do you think it would be possible to transfer the repo to the official asciidoctor github organization?

Absolutely. There's only one think I want to resolve before we do that. Does this extension replace (or supersede) the one by François-Xavier at https://github.com/fix/asciidoctor-screenshot?

If so, we should update the entry on the following page:

http://asciidoctor.org/docs/extensions/#third-party-extensions

Looking through the extensions on that page, I realize that we aren't naming the repositories consistently. What makes more sense to you?

* asciidoctor-screenshot

* asciidoctorj-screenshot

* asciidoctor-screenshot-extension

* asciidoctorj-screenshot-extension

* asciidoctor-extension-screenshot

* asciidoctorj-extension-screenshot

If we follow the lead of Asciidoctor Diagram, we'd choose asciidoctorj-screenshot, as you've already named it. (Naming is hard).

I still prefer the macro to be called "screenshot" and not capture.

That makes sense.

Reason for this is that all the other extensions I found also use nouns to describe what is included into the documentations (i.e. image::, link::, issue::, include::).

Great observation!

My only suggestion at this point is to name the attribute for the screenshot target directory screenshotsoutdir (or screenshotsdir) instead of screenshot-dir-name. It's still not the best name, but more consistent with other names in AsciiDoc like imagesdir (and imagesoutdir).

In fact, to align with Asciidoctor Diagram, it might even be better to use {imagesoutdir}/screenshots as the path. That way, the imagesoutdir only has to be set once. How about name the attribute "screenshotsdir" and make the default value "{imagesoutdir}/screenshots"?

Again, great work!

-Dan

On Tue, Mar 29, 2016 at 3:14 PM, sclassen [via Asciidoctor :: Discussion] <[hidden email]> wrote:

OK, I gave the naming a second though.

I still prefer the macro to be called "screenshot" and not capture.
Reason for this is that all the other extensions I found also use nouns to describe what is included into the documentations (i.e. image::, link::, issue::, include::).

For the browsing block I currently tend to call it "geb" because it contains GEB code. The reason is similar. I try to come up with a name describing what it is.

Feedback is more than welcomed :)

If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/AsciidoctorJ-Screenshot-Extension-tp4461p4561.html

To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML

Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen

sclassen

Re: AsciidoctorJ Screenshot Extension

Thanks Dan

I met Alex Soto this weekend at GreachConf in Madrid and we were talking about the screenshot extension.
I also gave a talk about how to write Asciidoctor extensions using Groovy.

But let's get back to the topic.

I open issues @github for the phantomJS/Chrome and the output dir configuration.

Regarding the extension of François-Xavier:
I started the current implementation by copying his code. I then converted his gradle build script into a standalone extension.
Afterwards I implemented most of the changes you suggested in the mailing list (http://discuss.asciidoctor.org/screenshot-plugin-for-asciidoctor-td2072.html).
Additionally I replaced the frames of François-Xavier with new images since I could not tell where these images came from and what copy right applies to them. In the same step I reduced the number of frames because some of the phones used are outdated today.

As a conclusion I would say the updated implementation supersedes the one of François-Xavier because it is a standalone extension and it does things more the Asciidoctor way.
The current drawback is that the number of frames has been reduced. And I kind of feel bad if François-Xavier entry gets replaced because he did most of the work for the screenshot extension.

About the naming of the repository. I definitely would go for asciidoctorj-screenshot. The 'j' is important to let everybody know that the extension needs a JVM. But adding the '-extension' does not deliver any more value.

Regards
Stephan

mojavelinux

Re: AsciidoctorJ Screenshot Extension

Administrator

Stephan,

Thank you very much to you and François-Xavier for your excellent work on the screenshot extension.

I also want to thank you for sharing what you learned about writing Asciidoctor extensions with the audience at GreachConf. Extensions are an important part of the Asciidoctor toolchain. The more people that understand how to leverage them, the more time writers will save and the better the documentation will become.

I'm proud to say that AsciidoctorJ Screenshot is now part of the Asciidoctor organization on GitHub as https://github.com/asciidoctor/asciidoctorj-screenshot

The issues you filed can now be found at https://github.com/asciidoctor/asciidoctorj-screenshot/issues

Even better, I was able to preserve the git history beginning with the work by François-Xavier, so no credit has been lost!

I'm honored to welcome you and François-Xavier to the Asciidoctor contributors page: http://asciidoctor.org/contributors/

Congratulations and thank you again for creating what is sure to become a widely-used extension in the Asciidoctor toolchain.

Cheers,

-Dan

p.s. Perhaps file the reduction in frames as an issue so that we can discuss it and decide whether to restore the previous behavior.

On Sun, Apr 10, 2016 at 5:23 PM, sclassen [via Asciidoctor :: Discussion] <[hidden email]> wrote:

Thanks Dan

I met Alex Soto this weekend at GreachConf in Madrid and we were talking about the screenshot extension.
I also gave a talk about how to write Asciidoctor extensions using Groovy.

But let's get back to the topic.

I open issues @github for the phantomJS/Chrome and the output dir configuration.

Regarding the extension of François-Xavier:
I started the current implementation by copying his code. I then converted his gradle build script into a standalone extension.
Afterwards I implemented most of the changes you suggested in the mailing list (http://discuss.asciidoctor.org/screenshot-plugin-for-asciidoctor-td2072.html).
Additionally I replaced the frames of François-Xavier with new images since I could not tell where these images came from and what copy right applies to them. In the same step I reduced the number of frames because some of the phones used are outdated today.

As a conclusion I would say the updated implementation supersedes the one of François-Xavier because it is a standalone extension and it does things more the Asciidoctor way.
The current drawback is that the number of frames has been reduced. And I kind of feel bad if François-Xavier entry gets replaced because he did most of the work for the screenshot extension.

About the naming of the repository. I definitely would go for asciidoctorj-screenshot. The 'j' is important to let everybody know that the extension needs a JVM. But adding the '-extension' does not deliver any more value.

Regards
Stephan

If you reply to this email, your message will be added to the discussion below:
http://discuss.asciidoctor.org/AsciidoctorJ-Screenshot-Extension-tp4461p4606.html

To start a new topic under Asciidoctor :: Discussion, email [hidden email]
To unsubscribe from Asciidoctor :: Discussion, click here.
NAML

Dan Allen | @mojavelinux | http://google.com/profiles/dan.j.allen