-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed issue #15 Upgrade to Asciidoctor 2.2.0 #17
Conversation
corrected typo
of standalone and header_footer option
package.json
npm information in README.md
@Mogztter I get an error if I try to run the test case with a converter that is exported as a class. I don't know, why that is the case. Do you have any idea? It says: When I export the class via a function, it works. |
@henriette-einstein Strange... we do have a test on this feature: https://github.com/asciidoctor/asciidoctor.js/blob/cd07a68127eff5cd080e9926c0a85ff388fcadc8/packages/core/spec/node/asciidoctor.spec.js#L2386 and the class definition is here: https://github.com/asciidoctor/asciidoctor.js/blob/cd07a68127eff5cd080e9926c0a85ff388fcadc8/packages/core/spec/node/asciidoctor.spec.js#L2175-L2202 Can you please share a sample reproduction case? And/or add a failing test? |
There is a test case included in this branch, that fails when you run "yarn test". It is the same as in the previous release (the master branch). It worked with Asciidoctor 2.1.2. To reproduce, you can just checkout this branch and run "yarn test" or "npm test". I can isolate that test and add some more stuff if that will help. Just give me notice. |
I just saw, that you are passing in a class. |
I tweaked the code a little for debugging. Now I get the following "stacktrace"
Another info: asciidoctor.ConverterFactory.register(new options.converter(), [asciidoctorOptions.backend]) and line 406 of gulp-asciidoctor-spec.js to
|
@henriette-einstein Thanks I will checkout your branch tomorrow to try to understand why it's failing. |
I found it! That is probably a pathological edge-case. What happened is basically
In that case, I have a class that is instantiated and registered before the class itself is passed as an option to the asciidoctor processor. I don't know if such a case could be checked in your code. I removed the option "converter" after the registration. Then it works. Sorry for the disturbances, it was my fault. |
asciidoctor.convert
I did not know, that "converter" was actually an option that is used by the underlying Asciidoctor. I therefore changed the name of the plugin option to "cnv" to avoid problems. |
Yes |
doc/documentation.adoc
Outdated
---- | ||
|
||
== Usage | ||
This plugin can be used to integrate Asciidoctor processing in yout gulp tasks. Here is a typical usage scenario. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This plugin can be used to integrate Asciidoctor processing in yout gulp tasks. Here is a typical usage scenario. | |
This plugin can be used to integrate Asciidoctor processing in your gulp tasks. Here is a typical usage scenario: |
doc/documentation.adoc
Outdated
exports.process = gulp.parallel(processAdocFiles, copyImages) | ||
---- | ||
|
||
You can configure the plugin itself by just adding options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can configure the plugin itself by just adding options | |
You can configure the plugin itself by adding options: |
doc/documentation.adoc
Outdated
... | ||
---- | ||
|
||
And you can also configure the underlying Asciidoctor processor via the respective Asciidoctor options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And you can also configure the underlying Asciidoctor processor via the respective Asciidoctor options | |
And you can also configure the underlying Asciidoctor processor via the respective Asciidoctor options: |
doc/documentation.adoc
Outdated
The following options can be used to configure the Gulp-Plugin | ||
|
||
extension:: Sets the output extension of the plugin. Defaults to '.html' | ||
cnv:: Sets a custom converter that should be used for processing. Defaults to undefined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cnv:: Sets a custom converter that should be used for processing. Defaults to undefined. | |
cnv:: Sets a custom converter that should be used for processing. Defaults to `undefined`. |
doc/documentation.adoc
Outdated
cnv:: Sets a custom converter that should be used for processing. Defaults to undefined. | ||
|
||
== AsciiDoctor Options | ||
All options of the underlying AsciiDoctor can be set. Please refer to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All options of the underlying AsciiDoctor can be set. Please refer to the | |
All options of the underlying Asciidoctor can be set. Please refer to the |
doc/documentation.adoc
Outdated
All options of the underlying AsciiDoctor can be set. Please refer to the | ||
https://asciidoctor-docs.netlify.app/asciidoctor.js/processor/convert-options[Converter options] page to see, which options are available. | ||
|
||
However, the Asciidoctor options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, the Asciidoctor options | |
However, the following Asciidoctor options: |
doc/documentation.adoc
Outdated
== Important | ||
|
||
=== Base Directory | ||
Do not forget to set the AsciiDoctor option `base_dir` if you want to include |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not forget to set the AsciiDoctor option `base_dir` if you want to include | |
Do not forget to set the Asciidoctor option `base_dir` if you want to include |
doc/documentation.adoc
Outdated
* If 'header_footer' is set and 'standalone' is not set, the processor will receive 'standalone' = value of 'header_footer' option and the option 'header_footer' will be stripped. | ||
* If both 'header_footer' and 'standalone' are set, the option 'header_footer' will be stripped. | ||
|
||
== Change Log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's one word (not 100% sure):
== Change Log | |
== Changelog |
@henriette-einstein The code looks fine, I only have minors nitpicks about the documentation, otherwise 👍 |
asciidoctor_2_2_0-branch
@Mogztter I am ready to merge the pull request if that is ok for you |
@henriette-einstein It looks good to me, I think you can merge it! Well done 👍 |
🎉 🎉 🎉 |
No description provided.