Project

General

Profile

Actions

Bug #14243

closed

Comments inside ERB tags broken

Added by jsc (Justin Collins) over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.5.0p0 (2017-12-25 revision 61468) [x86_64-linux]
[ruby-core:84484]

Description

A backwards-incompatible change was introduced with https://github.com/ruby/ruby/commit/abbfc048c5890e8017360bbc845062ea1585e155#diff-e1ea8366d3ac334deab7e0d25309be03

Example:

require "erb"
input = "<% # comment %>\n"
puts ERB.new(input).src

Before:

#coding:UTF-8
_erbout = +'';  # comment ; _erbout.<< -"\n"
; _erbout

After:

#coding:UTF-8
_erbout = +'';  # comment ; _erbout.<<(-"\n"
); _erbout

The generated code produces a syntax error due to introduction of parentheses around the arguments.


Files

erb_test.rb (71 Bytes) erb_test.rb Example script jsc (Justin Collins), 12/26/2017 07:34 PM

Related issues 1 (0 open1 closed)

Is duplicate of Ruby master - Bug #1868: ERB single line comment does not workRejectedk0kubun (Takashi Kokubun)Actions

Updated by hsbt (Hiroshi SHIBATA) over 4 years ago

  • Backport changed from 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN to 2.3: DONTNEED, 2.4: DONTNEED, 2.5: REQUIRED
  • Assignee set to k0kubun (Takashi Kokubun)
  • Status changed from Open to Assigned
Actions #2

Updated by k0kubun (Takashi Kokubun) over 4 years ago

  • Is duplicate of Bug #1868: ERB single line comment does not work added

Updated by k0kubun (Takashi Kokubun) over 4 years ago

  • Status changed from Assigned to Rejected

https://github.com/ruby/ruby/commit/abbfc048c5890e8017360bbc845062ea1585e155#diff-e1ea8366d3ac334deab7e0d25309be03 does trigger this behavior, but ERB does not support "<% #" syntax. Do you think the code generated at "Before:" is correct? It generates _erbout.<< -"\n" as comment but obviously it's not intended. I even think raising an error is better, but the behavior is out of scope for support as ERB level because embedding invalid syntax Ruby code is always invalid.

See also: https://bugs.ruby-lang.org/issues/1868. Use "<%#" instead of "<% #".

Updated by normalperson (Eric Wong) over 4 years ago

wrote:

Status changed from Assigned to Rejected

https://github.com/ruby/ruby/commit/abbfc048c5890e8017360bbc845062ea1585e155#diff-e1ea8366d3ac334deab7e0d25309be03 does trigger this behavior, but ERB does not support "<% #" syntax. Do you think the code generated at "Before:" is correct? It generates _erbout.<< -"\n" as comment but obviously it's not intended. I even think raising an error is better, and the behaivor is out of scope for support as ERB level because embedding invalid syntax Ruby code is always invalid.

See also Bug#1868. Use "<%#" instead of "<% #".

I understand your reasoning for the change; but I think
completely breaking something which was even half-working
should have a deprecation period of several years.

Otherwise existing users will be tempted to abandon ERb or even
Ruby (this happened with Ruby 1.9). Basically, this is why I
trust Linux kernel developers; they will jump through hoops to
not break userspace.

Actions #5

Updated by k0kubun (Takashi Kokubun) over 4 years ago

  • Status changed from Rejected to Assigned
Actions #6

Updated by k0kubun (Takashi Kokubun) over 4 years ago

  • Status changed from Assigned to Closed

Applied in changeset trunk|r61497.


erb.rb: preserve the behavior for invalid syntax

comment. Fix regression at r58948.

I even don't want to deprecate it because deprecation needs to lex all
embedded Ruby script using Ripper and it would be slow. So Let me just
keep this behavior of Ruby 2.4. No change is the best compatibility.

This commit stopped using String#-@ because it's harmful for "ambiguous
first argument" warning if we really want to maintain this behavior.

[Bug #14243]

Updated by k0kubun (Takashi Kokubun) over 4 years ago

As I don't have strong motivation to change the behavior, I fixed the behavior to be the same as Ruby 2.4 in r61497. Please use "<%#" for Ruby 2.5.0 and wait for Ruby 2.5.1.

Updated by jsc (Justin Collins) over 4 years ago

Thank you for reverting even though previous behavior was not 100% "correct".

Updated by naruse (Yui NARUSE) over 4 years ago

  • Backport changed from 2.3: DONTNEED, 2.4: DONTNEED, 2.5: REQUIRED to 2.3: DONTNEED, 2.4: DONTNEED, 2.5: DONE

ruby_2_5 r61581 merged revision(s) 61497.

Actions

Also available in: Atom PDF