inheriting from string is a design flaw

[stephen@…]

I want to set TabWidth=2. Without changing the source. So, I'm trying to make tabWidth an instance variable.

It took me way too long to find out what was going wrong, primarily because of this "inherit from string" design flaw.

For example, take a look at this snippet from to_html:

raise "tw1 not set" if tab_width.nil?
text = self.gsub( /\r\n?/, "\n" ).detab
raise "tw2 not set" if text.tab_width.nil?

So technically self.gsub(..).detab works because gsub makes sure to return a BlueCloth instance. So the method is there. However, all of the instance variable state is gone. tab_width is actually nil (not the default of 4 which I set in BlueCloth#initialize).

I hacked around this missing instance variable thing by doing:

text = self.gsub( /\r\n?/, "\n" )
text = detab(text)

Just like the other methods in to_html, e.g.

text = hide_html_blocks( text, rs )
...
text = strip_link_definitions( text, rs )

And so looking at this, the methods hide_html_blocks/strip_link_definitions are rather bastardized instance methods...they don't work against the BlueCloth<String's internal state (like most instance methods will do), but require a bunch of external state.

Which, don't get me wrong, I think your text = hide_html_blocks(text, rs) is great.

I think its just confusing to pretend a BlueCloth processor is a string. I don't think it should be...a BlueCloth processor will have its own non-string state (e.g. tab_width, rs, etc.) and work against many different strings and not just its own parent-class internal state.

Perhaps API compatibility with RedCloth? is paramount, but I think something like:

bc = BlueCloth.new(:tabWidth => 2)
bc.process("input1")
bc.process("input2")

(Or bc.to_html("input1"), I'm equivocal about the actual method name.)

Will make for a nicer separation of concerns between the processor and the "processee".

I would be willing to implement this refactoring if given the okay.

![Ed - wrapped the Ruby code examples in preproc blocks]

[deveiant]

Man, I totally missed this being opened. I apologize for my lack of response.

You make some good points, and I will seriously consider changing the String-inheritance thing. I wonder, in the meantime, if implementing the copy-initializer would preserve your instance variables. I'll give that a try, too, but I do prefer your proposed API.

Sticking this in the bugfix release, as it seems in retrospect like a bug to have subclassed String in the first place.

[deveiant]

It appears I'm going to start having some free time to work on BlueCloth again, so I'm going to try to apply all the bugfixes I've received and package up a new release.

This is one of the fixes I most want to get in, as it meshes with a bunch of other nice new features I'd like to support, but I'll have to be careful not to break backwards compatibility.