https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112015-11-24T19:38:48ZRuby Issue Tracking SystemRuby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=550632015-11-24T19:38:48Zsikachu (Prem Sichanugrist)s+ruby@sikac.hu
<ul></ul><p>I also just created a GitHub PR in case that will be easier to give feedback to the code: <a href="https://github.com/ruby/ruby/pull/1113" class="external">https://github.com/ruby/ruby/pull/1113</a></p>
<p>Thank you,<br>
Prem</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=550642015-11-24T19:41:59Zsikachu (Prem Sichanugrist)s+ruby@sikac.hu
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/55064/diff?detail_id=39456">diff</a>)</li></ul> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=550662015-11-24T20:48:18Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><blockquote>
<pre><code class="diff syntaxhl" data-language="diff"><span class="gi">+static VALUE
+rb_str_squish_bang(VALUE str)
+{
+ static const char before_regex_source[] = "\\A[[:space:]]+";
+ static const char after_regex_source[] = "[[:space:]]+\\z";
+ static const char between_regex_source[] = "[[:space:]]+";
+ VALUE before_argv[] = {
+ rb_reg_new(before_regex_source, sizeof before_regex_source - 1, 0),
+ rb_str_new_cstr("")
+ };
+ VALUE after_argv[] = {
+ rb_reg_new(after_regex_source, sizeof after_regex_source - 1, 0),
+ rb_str_new_cstr("")
+ };
+ VALUE between_argv[] = {
+ rb_reg_new(between_regex_source, sizeof between_regex_source - 1, 0),
+ rb_str_new_cstr(" ")
+ };
</span></code></pre>
</blockquote>
<p>You could memoize these Regexps as static variables and use<br>
<code>rb_gc_register_mark_object</code> to keep them around so GC won't eat them.<br>
Allocating 3 regexps and 3 strings every call seems like a waste.<br>
You may also use the same</p>
<p>Writing the equivalent Ruby code would only allocate the Regexps once.</p>
<p>You can also auto-dedupe <code>""</code> and <code>" "</code> strings with the magic<br>
"frozen_string_literal: true" comment in Ruby or<br>
<code>rb_fstring_cstr</code> function in C.</p>
<blockquote>
<p>By the way, this is my first patch and my first time writing something<br>
in C, so there might be something that does not look right to you.<br>
I'll happy to revise this patch (and learn about C in the process!)<br>
from your feedback.</p>
</blockquote>
<p>No worries; but personally (not speaking for the rest of ruby-core);<br>
I would prefer we use prelude.rb more and implement more things in Ruby<br>
rather than C.</p>
<p>Also (definitely not speaking for anybody else in ruby-core);<br>
but the Redmine <-> ruby-core ML integration is the only reason<br>
I've been willing to participate in Ruby development. I'm not touching<br>
proprietary websites or running any GUI/JavaScript at all to work on<br>
Ruby or any other Free Software.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=550782015-11-25T04:20:53Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/55078/diff?detail_id=39460">diff</a>)</li></ul><p>First, indented heredoc will be achieved by [Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Indent heredoc against the left margin by default when "indented closing identifier" is turned on. (Closed)" href="https://redmine.ruby-lang.org/issues/9098">#9098</a>], this will not be needed.</p>
<p>Second, <code>squish!</code> feels to return <code>nil</code> if no white spaces, to me, like as <code>sub!</code> and so on.</p>
<p>Last, it's prohibited in C89 to initialize an aggregate type by dynamic values.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=550892015-11-25T17:03:04Zdeivid (David RodrÃguez)
<ul></ul><p>I don't think the two features are the same, and would find both of them useful.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=552962015-12-07T07:50:41Zko1 (Koichi Sasada)
<ul><li><strong>Assignee</strong> set to <i>matz (Yukihiro Matsumoto)</i></li></ul> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=552972015-12-07T07:51:22Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul></ul><p><a href="https://github.com/ruby/ruby/compare/trunk...nobu:feature/11735-squish" class="external">https://github.com/ruby/ruby/compare/trunk...nobu:feature/11735-squish</a></p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=553102015-12-07T14:19:01Zsikachu (Prem Sichanugrist)s+ruby@sikac.hu
<ul></ul><p>nobu's patch seems to be the better way to implement this feature without having to use regular expression. Much more efficient.</p>
<p>I guess I should try to think outside the box the next time I try to write something in C.</p>
<p>+1 to nobu's patch. Thank you for your hard work.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=587232016-05-18T01:22:31Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>We looked at it on yesterday's developer meeting but didn't reach a consensus. This might be because the attendees are mostly Hanzi culture residents (they treat newlines and spaces differently). This doesn't mean an immediate NG but frankly, we need to study real-world use cases more.</p>
<p>Note #1: recent (2.3+) ruby has squiggly heredoc ("<<~"). This can save someone who originally used squish.</p>
<p>Note #2: as we discuss, I reached a conclusion that squish over SQL is dangerous. It can destroy SQL's string literals with spaces.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=587482016-05-19T17:57:01Zgurgeous (Adam Doppelt)amd@gurge.com
<ul></ul><p>It would be great to include squish in String. I've been writing production Ruby code for years and I often pull in ActiveSupport just to get squish.</p>
<p>Getting input from a user? squish<br>
Cleaning up an iffy array.join? squish<br>
Pulling data from a web crawl? squish<br>
Normalizing concatenated data? squish</p>
<p>Squish squish squish. 60k hits on github for Ruby squish. Just squish it. Squish it into core, please.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=587602016-05-20T02:38:01Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>Adam Doppelt wrote:</p>
<blockquote>
<p>Getting input from a user? squish</p>
</blockquote>
<p>I guess you have never had a user from CJKV cultures.</p>
<blockquote>
<p>Cleaning up an iffy array.join? squish</p>
</blockquote>
<p>This is a huge NO. It destroys JSON.</p>
<blockquote>
<p>Pulling data from a web crawl? squish</p>
</blockquote>
<p>Also NO. It destroys <pre>.</p>
<blockquote>
<p>Normalizing concatenated data? squish</p>
</blockquote>
<p>This could be OK depending on the "normalization" you want.</p>
<blockquote>
<p>Squish squish squish. 60k hits on github for Ruby squish. Just squish it. Squish it into core, please.</p>
</blockquote>
<p>I had no pro et contra to the proposal until now. From what you said I started thinking squish can be a bad smell of indiscreet data treatment.</p>
<p>I hope I'm wrong.</p> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=593242016-06-23T17:33:15Zgurgeous (Adam Doppelt)amd@gurge.com
<ul></ul><p>Shyouhei Urabe wrote:</p>
<blockquote>
<p>I had no pro et contra to the proposal until now. From what you said I started thinking squish can be a bad smell of indiscreet data treatment.</p>
</blockquote>
<p>I think you're missing the point here. Squish is used to cleanup whitespace. If you want to preserve whitespace, don't call it. There are many, many, many (many) situations where cleaning up whitespace is desirable and squish is perfect.</p>
<p>Yes, I actually need to destroy newlines, smash spaces together, and annihilate </p><pre> tags. This is incredibly common. That's why I use squish. If I didn't want to do those things I would not use squish.</pre> Ruby master - Feature #11735: Porting String#squish and String#squish! from Ruby on Rails' Active Supporthttps://redmine.ruby-lang.org/issues/11735?journal_id=593392016-06-24T08:02:10Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>I don't stop you to do what you want.</p>
<p>But you are requesting it into core. There are so many miserably misused APIs around the world. Isn't it just another example of such thing? Isn't it "too easy to fail"? For instance is it OK say that applying this method to user input wont introduce SQL injection or that sort of things?</p>