Skip to content

Fix "instance variable @options not initialized" warning in verbose mode#650

Merged
robin850 merged 1 commit intovmg:masterfrom
kolen:fix-warning-options-not-initialized
Mar 25, 2018
Merged

Fix "instance variable @options not initialized" warning in verbose mode#650
robin850 merged 1 commit intovmg:masterfrom
kolen:fix-warning-options-not-initialized

Conversation

@kolen
Copy link
Contributor

@kolen kolen commented Feb 28, 2018

When running in verbose mode (-w or --verbose or $VERBOSE = true), when instantiating any renderer (subclass of Redcarpet::Render::Base) without parameters, warning is shown about @options not initialized:

warning: instance variable @options not initialized

(For example, it can be seen if running RUBYOPT=-w rake test)

It's caused by accessing @options with rb_iv_get:

if (rb_iv_get(self, "@options") == Qnil)
rb_iv_set(self, "@options", rb_hash_new());

Replaced it with rb_attr_get, according to source, it's the same as rb_ivar_get but without warnings and without conversion of undef to nil, not sure if it's the right method by design (seems that it is).

Also not sure what is purpose for @options in render objects: there are no reads of it and all options are passed to actual renderer in constructor only, overwriting @options later is meaningless. Redcarpet::Markdown object adds its options to it when initialized but I can't figure out where these options are used (and these options are not for renderer but for "extensions"). Update: it's feature #560 and intended for custom renderers to be able to access "extension options" and not only renderer options passed in constructor.

I can try to detect warnings in tests, but I'm not sure if it's right idea (it can only be done in quirky ways such as capturing stderr), so not changed tests for now.

Copy link

@fedotov2d fedotov2d left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@kolen kolen force-pushed the fix-warning-options-not-initialized branch from 238fa7d to 6b86656 Compare March 7, 2018 15:56
@robin850 robin850 merged commit e23383e into vmg:master Mar 25, 2018
@robin850
Copy link
Collaborator

Thank you very much @kolen ! :-)

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.

3 participants