Added new feature to fail based on different conditions #363
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abelsromero
force-pushed
the
add_capture_of_Asciidoctor_log_messages
branch
from
37b32b2
to
20f4c02
Compare
|
Added README, no more changes to make. |
|
no comments? cc @mojavelinux |
|
Hello, Overall, I'd be interested in having the build fail whenever there's a missing file to include, image to embed, etc. I understand that the feature addressed by this PR could help me do what I want, short of being able to configure asciidoctor to emit ERRORS instead of WARNINGS. Just so you know there are people around here watching the progress of the Pull Request ;-) |
|
Hey, just built locally and tried the new feature (why haven't I done so earlier? dunno…) The On a side note, the
|
|
Well, maybe I spoke a little bit too early @abelsromero : when I stop deliberately putting errors (e.g. including inexistant files) in my asciidocs, then I expect the build to succeed. |
|
I like the proposal and I'm glad you decided to move forward with it. I think the If there are improvements to be made, I agree we should handle them as discrete issues. You don't want an issue to get so huge it never gets delivered. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses: #261, #178
Features are complete, but there's an issue with duplicated asciidoctor messages asciidoctor/asciidoctorj#669. Depending on suggestions we can merge or review the feature.
Feature summary
This PR adds a new configuration element as follows:
outputToConsole: redirects asciidoctor messages to maven's logger. Messages are shown as Maven's INFO during renderitzation. Messages will appear twice as mentioned in issue Unable to prevent messages from showing in console using LogHandler asciidoctorj#669.severity: named so to align with AsciidoctorJ property. Will make the build fail if a message of this severity level is found.It only matches the exact level, not same and above. I did this for simplicity but I am open to suggestions, implementation is trivial.I finally decided to filter all messages above certain level to allow capturing all messages easily, also, feels more natural to understand.containsText: named so instead ofmatchesTextbecause it just asserts that, acontains. I considered the option of allowing regex, but this made configuration more complex and less expressive for simple cases like a simple contains. Adding regex options could be done with another option or some custom syntax in the field.Note that if both
severityandcontainsTextare set, only messages that match both are captured.Messages that match
severityand/orcontainsTextare shown as Maven ERROR (independently fromoutputToConsole), so that CIs can capture them as cause of build fail. Also, an initial error message showing the number of issues found is returned.If someone wants to create a custom report like mentioned in #261, you can register a custom LogHandler using the SPI interface (added integration test for that). However, this has some limitations with some formats that need to "close" and estructure (see asciidoctor/asciidoctor#2832).
Other notes
severityandcontainsTextare validated after the renderitzation the target document (html, pdf, etc.) can be found in the target directory. I don't see any reason to delete it.Other changes to code
InlineMacroProcessortest extension to new especificationjruby-completeto 9.1.16.0: updatedmaven-plugin-pluginfrom 3.4 to 3.5 to fix issue after updatingjruby-completeI kept code as Java 7 compatible, but AsciidoctorJ is 1.8. I'd add that to docs and release notes when PR is agreed. I did not want to add more things to the mix.