Skip to content
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

Disable automatic logging of invalid refs and enable it via verbose m… #689

Merged
merged 2 commits into from
Oct 11, 2018

Conversation

robertpanzer
Copy link
Member

…ode from CLI

This PR disables the automatic activation of the verbose mode again and thus disables logging of invalid refs.

The -v argument for the CLI additionally enables this again.
Therefore integrators have to run the Ruby script $VERBOSE=true themselves before rendering documents.

@mojavelinux Is this what you had in mind?

@mojavelinux
Copy link
Member

Exactly.

@@ -135,6 +136,9 @@ public void log(LogRecord logRecord) {
};
asciidoctor.registerLogHandler(logHandler);

// Asciidoctor currently only logs invalid refs if this global var is set
JRubyRuntimeContext.get().evalScriptlet("$VERBOSE=true");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to be unset again after the test is run? Or do tests not share the JRuby runtime context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fine, hopefully.
Every Asciidoctor instance gets its own JRuby instance and should have it's own global state if I am not wrong.
But definitely sth to retest.

@robertpanzer
Copy link
Member Author

Added a second test to ensure that invalid refs log messages are not created by default.

I don't really like it, but also ensured that this test runs after the first that shows that $VERBOSE results in these messages being created to prove that the $VERBOSE global variable doesn't leak into other Asciidoctor instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants