Project

General

Profile

Actions

Feature #6265

open

Remove 'useless' 'concatenation' syntax

Added by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago. Updated over 7 years ago.

Status:
Assigned
Target version:
-
[ruby-core:44156]

Description

What is wrong with this code:

some_method 'argument1', 'argument2' 'argument3'

Yes, the missing colon, but it is not always easy to notice that...

What is this ('concatenation' 'syntax') useful for?

Why writing ('some ' 'concatenation') instead of 'some concatenation'?

A missing colon between string arguments can lead to some bugs that may be hard to find, specially if the arguments are optional.

And I can't see any useful case where this allowed syntax for concatenation would help.

Updated by now (Nikolai Weibull) almost 12 years ago

On Fri, Apr 6, 2012 at 14:53, rosenfeld (Rodrigo Rosenfeld Rosas)
wrote:

What is this ('concatenation' 'syntax') useful for?

raise ArgumentError,
'they are useful for long strings that are so long that they need '
'to span multiple lines to fit within a reasonable line width'

Updated by marcandre (Marc-Andre Lafortune) almost 12 years ago

  • Target version changed from 3.0 to 2.0.0

Hi,

now (Nikolai Weibull) wrote:

On Fri, Apr 6, 2012 at 14:53, rosenfeld (Rodrigo Rosenfeld Rosas)
wrote:

What is this ('concatenation' 'syntax') useful for?

raise ArgumentError,
'they are useful for long strings that are so long that they need '
'to span multiple lines to fit within a reasonable line width'

In this example, replacing the "" with a "+" will give basically the same result without using the concatenation rule.

Are there valid use cases where two strings are concatenated on the same line? I think that's really what the request is all about, i.e. forbid that kind of concatenation for two strings on the same line.

Updated by mame (Yusuke Endoh) almost 12 years ago

  • Status changed from Open to Assigned
  • Target version changed from 2.0.0 to 3.0

msg = 'put "\n" here -> ' "\n"

I found an actual example in lib/mkmf.rb:

hdr = ['#include "ruby.h"' "\n"]

Anyway, 2.0 should remain all existing features unless there is a good reason.

--
Yusuke Endoh

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago

I found an actual example in lib/mkmf.rb:

hdr = ['#include "ruby.h"' "\n"]

Wouldn't this read better this way?

hdr = [%Q{#include "ruby.h"\n}]

Updated by mame (Yusuke Endoh) almost 12 years ago

I don't think the current code in mkmf.rb is good.
I just meant that the syntax is actually used and thus we cannot remove it from 2.0.
I'm not against this proposal in the future.

$ cat concat_quine.rb
eval$s="s='eval$s='+$s.split.join.dump;('%0382b'%(('40i3tgbj1q4n'+
'"       "n3"       "t7"      "1py"       "1ur"     "h'"        "+
'"  "xtb74az"  "4"  "ym"  "2"  "03"  "fx542i6"  "e"  "udoj"  "').#
t"  "o_i(36)"  "<"  "<7"  "0"  "))"  ".gsub(/"  "0"  "(?=1"  ")|xx
|"  "1(?=0)/"  ")"  "{$"  "&"  "<<"  "34}.tr("       "'1',"  "''<<
3"       "2)"       ".g"  "s"  "ub"       "('"  "0"  "'){;"  ";;s.
slice!(0,1)}.scan(/.{66}/){puts$&}#_CONCAT_QUINE_(c)_Y.Endoh_2012"

$ ruby concat_quine.rb
eval$s="s='eval$s='+$s.split.join.dump;('%0382b'%(('40i3tgbj1q4n'+
'"       "n3"       "t7"      "1py"       "1ur"     "h'"        "+
'"  "xtb74az"  "4"  "ym"  "2"  "03"  "fx542i6"  "e"  "udoj"  "').#
t"  "o_i(36)"  "<"  "<7"  "0"  "))"  ".gsub(/"  "0"  "(?=1"  ")|xx
|"  "1(?=0)/"  ")"  "{$"  "&"  "<<"  "34}.tr("       "'1',"  "''<<
3"       "2)"       ".g"  "s"  "ub"       "('"  "0"  "'){;"  ";;s.
slice!(0,1)}.scan(/.{66}/){puts$&}#_CONCAT_QUINE_(c)_Y.Endoh_2012"

Updated by now (Nikolai Weibull) almost 12 years ago

On Fri, Apr 6, 2012 at 16:52, marcandre (Marc-Andre Lafortune)
wrote:

 > What is this ('concatenation' 'syntax') useful for?

 raise ArgumentError,
   'they are useful for long strings that are so long that they need '
   'to span multiple lines to fit within a reasonable line width'

In this example, replacing the "" with a "+" will give basically the same result without using the concatenation rule.

The “basically the same” being that the first happens at compile time
and the second at run time, for anyone wondering.

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago

Yusuke, am I supposed to understand what you meant with the code in your last comment? If so, I didn't get it...

Nikolai, good point, I haven't thought about this before and I wasn't wondering :)

But do you really think that this could have some meaningful impact in the performance of replacing '' with '+'?

Updated by matz (Yukihiro Matsumoto) almost 12 years ago

  • Assignee changed from matz (Yukihiro Matsumoto) to mame (Yusuke Endoh)

I agree with removing string concatenation in 3.0.

Updated by mame (Yusuke Endoh) almost 12 years ago

Matz,

I agree with removing string concatenation in 3.0.

Okay. Then, is the syntax deprecated in 2.0?
If so, it would be good to warn it.

diff --git a/parse.y b/parse.y
index 0393dc1..1726117 100644
--- a/parse.y
+++ b/parse.y
@@ -3916,6 +3916,7 @@ string		: tCHAR
		| string string1
		    {
		    /*%%%*/
+			rb_warning0("string concatenation syntax is deprecated");
			$$ = literal_concat($1, $2);
		    /*%
			$$ = dispatch2(string_concat, $1, $2);

Rodorigo,

Yusuke, am I supposed to understand what you meant with the code in your last comment? If so, I didn't get it...

Sorry for the confusion. The code is just a joke.
I can write such a code much easily without the syntax.

eval$s=%q(puts(%(eval$s=%q(#$s)))#################################
##       ####       ####     ######       #####     ####        ##
##  #########  ###  ####  ##  #####  #########  ###  ######  #####
##  #########  ###  ####  ##  #####  #########  ###  ######  #####
##  #########  ###  ####  ##  #####  #########       ######  #####
##       ####       ####  ##  #####       ####  #l#  ######  #####
#################################################################)

--
Yusuke Endoh

Updated by matz (Yukihiro Matsumoto) almost 12 years ago

Hi,

In message "Re: [ruby-core:44207] [ruby-trunk - Feature #6265] Remove 'useless' 'concatenation' syntax"
on Mon, 9 Apr 2012 12:43:58 +0900, "mame (Yusuke Endoh)" writes:

I agree with removing string concatenation in 3.0.

Okay. Then, is the syntax deprecated in 2.0?
If so, it would be good to warn it.

I agree with making it deprecated. It is up to you to make it warn or
not.

						matz.

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago

Thank you very much! I do also agree that we should warn users so that they have some time to change their "" style to "+".

Also, while on the subject, don't you think that Ruby specs should state that ("some " + "string") would concatenate the strings in compiling time and that they shouldn't expect the :+ method to be called? That way we wouldn't have any perfomance degradation using this style...

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago

Hi Yusuke, I guessed it was a joke, but I couldn't get it at that time. Actually I had a hard time reading "CONCAT" in your last example. After that, I could read it too in the first ones, which is much harder :) I guess I have some problem with my brain! :D

Updated by nobu (Nobuyoshi Nakada) almost 12 years ago

Hi,

(12/04/09 22:46), rosenfeld (Rodrigo Rosenfeld Rosas) wrote:

Also, while on the subject, don't you think that Ruby specs should
state that ("some " + "string") would concatenate the strings in
compiling time and that they shouldn't expect the :+ method to be
called? That way we wouldn't have any perfomance degradation using
this style...

It can't, because String#+ might be overridden.

--
Nobu Nakada

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago

Em 09-04-2012 12:53, Nobuyoshi Nakada escreveu:

Hi,

(12/04/09 22:46), rosenfeld (Rodrigo Rosenfeld Rosas) wrote:

Also, while on the subject, don't you think that Ruby specs should
state that ("some " + "string") would concatenate the strings in
compiling time and that they shouldn't expect the :+ method to be
called? That way we wouldn't have any perfomance degradation using
this style...
It can't, because String#+ might be overridden.

I know, that is why I proposed this to be changed in a specification level.

For instance, what would be the issue with an overridden String#+ not
being called in such cases?

Updated by ko1 (Koichi Sasada) almost 12 years ago

(2012/04/09 14:19), Yukihiro Matsumoto wrote:

I agree with making it deprecated. It is up to you to make it warn or
not.

I prefer current behavior because it is same as C and it is easy to
remember.

Why you remove it? I think the proposed reasons before don't make sense
for me.

--
// SASADA Koichi at atdot dot net

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) almost 12 years ago

Em 09-04-2012 16:06, SASADA Koichi escreveu:

(2012/04/09 14:19), Yukihiro Matsumoto wrote:

I agree with making it deprecated. It is up to you to make it warn or
not.
I prefer current behavior because it is same as C and it is easy to
remember.

Why you remove it? I think the proposed reasons before don't make sense
for me.

Hi Koichi, I created this issue because I did have this problem.

I've hit some key by mistake in my Vim editor and it deleted the comma
separating two string arguments and it took me a while to figure it out
what have happened.

Updated by ko1 (Koichi Sasada) almost 12 years ago

(2012/04/10 7:01), Rodrigo Rosenfeld Rosas wrote:

Hi Koichi, I created this issue because I did have this problem.

I've hit some key by mistake in my Vim editor and it deleted the comma
separating two string arguments and it took me a while to figure it out
what have happened.

On the other hand, I have never met such mistake. So I want to know
such mistake should be considered or not.

--
// SASADA Koichi at atdot dot net

Updated by duerst (Martin Dürst) almost 12 years ago

ko1 (Koichi Sasada) wrote:

I prefer current behavior because it is same as C and it is easy to
remember.

It is more important in C because it can help with macros (#define). Fortunately, we don't have macros in Ruby, so it's less important in Ruby.

Why you remove it? I think the proposed reasons before don't make sense
for me.

I agree that there's no urgent need to remove it.

Updated by alexeymuranov (Alexey Muranov) almost 12 years ago

I think it is nice to have a syntax to concatenate strings in compile time. I do not think that missing commas are hard to notice.

Updated by trans (Thomas Sawyer) almost 12 years ago

The only time I have found it useful is with \ for multi-line string, for things like entry in Ruby based-config file. E.g.

self.description = 'This is a multi-line ' \
                   'description of something.'

Updated by trans (Thomas Sawyer) almost 12 years ago

trans (Thomas Sawyer) wrote:

The only time I have found it useful is with \ for multi-line string, for things like entry in Ruby based-config file. E.g.

self.description = 'This is a multi-line' \
                   'description of something.'

Updated by Anonymous almost 12 years ago

I agree with Alex that when I first saw it I thought it was cool, but then I never really needed to use it, like trans says.

Updated by marcandre (Marc-Andre Lafortune) over 11 years ago

  • Category set to core
  • Target version changed from 3.0 to 2.0.0

Changing target for 2.0 so that we add the deprecation warning.

A warning is good practice and should happen at the best moment: at parse time.

Actions #25

Updated by usa (Usaku NAKAMURA) over 11 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r37317.
Rodrigo, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


Updated by usa (Usaku NAKAMURA) over 11 years ago

  • Status changed from Closed to Assigned
  • Assignee changed from mame (Yusuke Endoh) to drbrain (Eric Hodel)

Now I've committed mame-san's patch (I was asked by him to commit instead of him).

But this change introduced some failures to make test-all, because rubygems
uses string concatenation at lib/rubygems.rb:1246-1249.
Eric, could you change these lines?

Actions #27

Updated by naruse (Yui NARUSE) over 11 years ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r37327.
Rodrigo, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


Revert r37316

The commit introduces too many failures and disturbs release engineering.
Re-commit it with fixed tests.

Thu Oct 25 13:09:01 2012 NAKAMURA Usaku

   * parse.y: show a warning for concatenating string literals because
     it will be deprecated in the future.
     patched by mame (Yusuke Endoh) at [ruby-core:44207].
     [ruby-core:44156] [Feature #6265]

Updated by naruse (Yui NARUSE) over 11 years ago

  • Status changed from Closed to Assigned

I reverted r37316 because such too many failures prevents watching CI.
Please re-commit it with fixes for those tests.
http://u32.rubyci.org/~chkbuild/ruby-trunk/log/20121025T110301Z.log.html.gz

Updated by naruse (Yui NARUSE) over 11 years ago

  • Status changed from Assigned to Feedback
  • Assignee deleted (drbrain (Eric Hodel))

Since affected libraries are so many, the fix should be done by the proposer.

Moreover I think this insists that string concatenation is not useless.

Updated by nobu (Nobuyoshi Nakada) over 11 years ago

I also strongly object this change, since it needed many escapes which were unnecessary otherwise.

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) over 11 years ago

Right now I'm in the middle of some very tight deadlines in my job and won't have time to fix all usages of this kind of string concatenation but I'd be happy to do so next year, although I'm afraid I wouldn't be on time for Ruby 2.0 release :(

Updated by drbrain (Eric Hodel) over 11 years ago

I don't think it is appropriate to include this change in the ruby 2.x series without any deprecation period. I have many libraries that depend on it.

Removing it in 3.0 with a warning in 2.x would be more appropriate.

Updated by ko1 (Koichi Sasada) over 11 years ago

  • Assignee set to mame (Yusuke Endoh)

Updated by mame (Yusuke Endoh) over 11 years ago

Eric,

drbrain (Eric Hodel) wrote:

Removing it in 3.0 with a warning in 2.x would be more appropriate.

We are actually trying to do so. r37316 is a warning patch for that.
And the warning, not changing syntax, caused many test failures in rubygems.

--
Yusuke Endoh

Updated by mame (Yusuke Endoh) over 11 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from mame (Yusuke Endoh) to drbrain (Eric Hodel)

drbrain, do you have no intention of fixing rubygems? We cannot add the warning.

(Or please make matz change his mind ;-)

--
Yusuke Endoh

Updated by drbrain (Eric Hodel) over 11 years ago

I will fix RubyGems and RDoc, please stand by.

Updated by mame (Yusuke Endoh) over 11 years ago

Thanks drbrain!

--
Yusuke Endoh

Actions #39

Updated by drbrain (Eric Hodel) over 11 years ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r38230.
Rodrigo, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • lib/rubygems/command_manager.rb: Removed string concatenation
    syntax. [Bug #6265]
  • lib/rubygems/commands/install_command.rb: ditto
  • lib/rubygems/commands/uninstall_command.rb: ditto
  • lib/rubygems/indexer.rb: ditto
  • lib/rubygems/security/policy.rb: ditto
  • lib/rubygems/security.rb: ditto
  • lib/rubygems/uninstaller.rb: ditto
  • test/rubygems/test_gem_commands_cert_command.rb: ditto
  • test/rubygems/test_gem_package.rb: ditto
  • test/rubygems/test_gem_security.rb: ditto
  • test/rubygems/test_gem_security_policy.rb: ditto

Updated by drbrain (Eric Hodel) over 11 years ago

  • Status changed from Closed to Assigned
  • Assignee changed from drbrain (Eric Hodel) to mame (Yusuke Endoh)

Oops, it was accidentally closed.

I have removed the string concatenation syntax from rdoc and rubygems.

Updated by mame (Yusuke Endoh) about 11 years ago

  • Subject changed from Remove 'useless' 'concatenation' syntax to Remove &#x27;useless&#x27; &#x27;concatenation&#x27; syntax
  • Target version changed from 2.0.0 to 2.6

drbrain, thanks!

But sorry, it is too late to change 2.0.0. My bad.

--
Yusuke Endoh

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) over 9 years ago

Yusuke, how about for 2.2.0?

Updated by nagachika (Tomoyuki Chikanaga) over 9 years ago

  • Assignee changed from mame (Yusuke Endoh) to naruse (Yui NARUSE)

Release Manager of 2.2.0 is naruse san.

Updated by nobu (Nobuyoshi Nakada) over 9 years ago

I'm against it, it's not "useless" to me.

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) over 9 years ago

I said "useless" in the sense that you can achieve the same using "+" and it makes it easy to introduce common bugs in case you forget a comma between two strings when calling a method and this has happened already in real code, including the Rails source code despite the test suite.

Updated by naruse (Yui NARUSE) over 9 years ago

  • Target version changed from 2.6 to 3.0

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

We looked at this issue at yesterday's developer meeting. This feature was once implemented, and then removed due to test failures.

The current situation is, it still renders lots of warnings when deprecation turned on. This string concatenation feature is used than people might think. It's still difficult to merge.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0