search

mhuggins / dogeify

Ruby gem for converting English to Doge.
MIT License
28 stars 5 forks source link

Fixed input that contains color codes #2

Closed Addisonbean closed 10 years ago

Addisonbean commented 10 years ago

I used your tutorial on this to make a gem of my own (my first actually) and it uses this gem. But it acts strange when color codes are used so I have fixed that. My gem is called muchdoge by the way, and it re-defines puts to make all output using puts dogeified! It's more of a prank than something useful.

mhuggins commented 10 years ago

Ahh, yeah, I could see this being an issue. This seems like a very specific issue to your use case, and I wonder if we could generalize it a bit more. Perhaps it would be nice to change the method signature from process(str) to process(str, options = {}), and allow something like this:

str = "some string with hex like #00ffdd"
ignored_patterns = [
    /\e\[(\d+)(;\d+)*m/
]

str = dogeifier.process(str, ignore: ignored_patterns)

I'll try to take a look at it this week, unless it's something you think you'd like to do. Does this sound like a reasonable middle-ground?

Addisonbean commented 10 years ago

Ya I'd love to do this!

Addisonbean commented 10 years ago

I added the ignore option for Dogeify#process

Addisonbean commented 10 years ago

Is this ever gonna get merged?

mhuggins commented 10 years ago

Sorry for the delay @Addisonbean, I've been busy (I have a full-time job and a family). I took a look earlier this morning and have some suggestions, but I didn't get a chance to write them out. I will as soon as I have some free time. Expect feedback later today or tomorrow. :)

mhuggins commented 10 years ago

Okay, I went ahead and added some feedback. Just a couple things to tidy up, and do you think you could squash the commits? Thanks for contributing! :)

Addisonbean commented 10 years ago

I fixed those last couple of things, thanks for the feedback!

mhuggins commented 10 years ago

Thanks, @Addisonbean! Any chance you could squash the commits? Happy to merge in after that, and I'll get another release out!

Addisonbean commented 10 years ago

Ok, done

mhuggins commented 10 years ago

Thanks for your contribution! It's merged in. I'm planning to make an update to allow the core extension methods to accept options as well to be forwarded to the Dogeify#process method, and then I'll push a new release out. Hoping to find time to do so today.

Addisonbean commented 10 years ago

Awesome, thanks!

mhuggins commented 10 years ago

Pushed out version 1.1.0 that includes this change. :) Thanks again!