Twitter: Preserve commit message whitespace #1075
Conversation
When tweeting commit messages, preserve the original commit message
whitespace and tweet it as-is.
Previously, String#split(" ") was used to process words, which splits on
*any* whitespace and with runs of contiguous whitespace characters
ignored [1]. When the split array was turned back into a string with
Array#join(" "), original whitespace characters were lost and replaced
by a single space character.
Now, String#gsub and a regular expression is used to process words,
which will preserve original whitespace.
Fixes #959.
[1] http://ruby-doc.org/core-2.2.0/String.html#method-i-split
|
This looks reasonable to me! @ptoomey3 can you take a look at this for a 👍? A regex changed but I'm pretty confident, since it is only changing how we escape at-mentions, that it is good to go. |
lib/services/twitter.rb
Outdated
There was a problem hiding this comment.
Total nit, but would [[:graph:]] be more accurate here? For example:
"foob @\tbar".gsub(/\B@[^ ]+/) { |word| puts word; "@X#{word[1..word.length]}" }
@ bar
=> "foob @X\tbar"vs
"foob @\tbar".gsub(/\B@[[:graph:]]+/) { |word| puts word; "@X#{word[1..word.length]}" }
=> "foob @\tbar"There was a problem hiding this comment.
You're right, [^ ] is definitely too specific. I looked at what Twitter themselves look for when matching mentions, and it looks like the @ must be followed by [a-zA-Z0-9_]{1,20}.
I changed the regex to use [^[:word:]] and added tests for this behavior.
Change regex to be more specific and look for the "@" symbol followed by a word character instead of any non-space character. Add tests for this behavior. See https://github.com/github/github-services/pull/1075/files#r37456732
Twitter matches the U+FF20 character (＠) in addition to the normal ASCII @ character. See https://github.com/twitter/twitter-text/blob/141759ce97926772d02c257dead8d27379d16dbb/rb/lib/twitter-text/regex.rb#L118
|
I added a commit to address @ptoomey3's comment. I also discovered that Twitter matches Edit: Let me know if you want me to squash these commits. |
Don't need to match the entire word, just the first word character after the `@`.
Twitter: Preserve commit message whitespace
|
Thanks @parshap for getting this fixed up! I'll get this deployed in the next day or two. 🍰 |
|
<3 <3 <3 @parshap |
4 participants
When tweeting commit messages, preserve the original commit message whitespace and tweet it as-is.
Previously,
String#split(" ")was used to process words, which splits on any whitespace and with runs of contiguous whitespace characters ignored. When the split array was turned back into a string withArray#join(" "), original whitespace characters were lost and replaced by a single space character.Now,
String#gsuband a regular expression is used to process words, which will preserve the original whitespace.Fixes #959.
/cc @domenic @kdaigle