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

Allow specifying hyphen character. #4

Merged
merged 2 commits into from
Jun 21, 2012

Conversation

e2
Copy link
Contributor

@e2 e2 commented Apr 29, 2012

Hi!

Patch (with tests) that allows user to specify something other than '-' (hyphen-minus) for hyphen.

Tested on Ruby 1.9.3 and 1.8.7.

I was wondering if this belongs here. I believe it does, since prawnpdf/prawn expects the soft-hyphen "\u00AD" instead.

Sure, users could just use:

.gsub('-', "\u00AD")

but that would need to be done on every word, even when the result is already cached by text-hyphen.

Also, people often use the minus sign because it's on the keyboard, when they really mean one of:

  • hyphen bullet ("\u2043") [yes, I just "sinned" too, but markdown forgives me ...]
  • non-breaking hyphen ("\u2011")
  • unicode minus sign ("\2212")
  • a ... um ... hyphen ("\2010")

Sorry for the rant, I just wanted to show I did my homework and some thinking.

@halostatue
Copy link
Owner

Good patch, for the most part. I'm not fond of the version detection in the test, though.

Yes, it should work with \u00ad, but why not try it with "­" as the string instead of the Unicode value? That way, you've got a multibyte string that you don't have to check whether you're running in Ruby 1.8 or Ruby 1.9.

Finally, please update the documentation for the methods (and maybe even provide a new example in the README) and this check-in is a slam-dunk for merge. I like it!

@e2
Copy link
Contributor Author

e2 commented Apr 30, 2012

Your comments made sense, thanks.

I thought about adding an '-s' option to ruby-hyphen to see if it would make sense, but there's missing method:

$ruby-hyphen -H 5 backpack 

/home/me/.rvm/gems/ruby-1.9.3-p194/gems/text-hyphen-1.2/bin/ruby-hyphen:98:in `block in <top (required)>': undefined
method `visualise_to' for #<Text::Hyphen:0x00000002924990> (NoMethodError)

So, I just gave up with that idea.

@e2
Copy link
Contributor Author

e2 commented May 4, 2012

Let me know when you merge this, so I can nuke my fork. I already have enough forks to open up a restaurant...

@halostatue halostatue closed this Jun 21, 2012
@ghost ghost assigned halostatue Jun 21, 2012
@halostatue halostatue reopened this Jun 21, 2012
halostatue added a commit that referenced this pull request Jun 21, 2012
Allow specifying hyphen character.
@halostatue halostatue merged commit 8a35678 into halostatue:master Jun 21, 2012
@halostatue
Copy link
Owner

And merged.

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.

None yet

2 participants