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

Bug in split with a positive lookahead assertion #1690

Closed
mitchty opened this issue Apr 29, 2012 · 6 comments
Closed

Bug in split with a positive lookahead assertion #1690

mitchty opened this issue Apr 29, 2012 · 6 comments

Comments

@mitchty
Copy link

mitchty commented Apr 29, 2012

Seems to be introducing extraneous blank elements when used with split.

Example CamelCase=>snake_case convertor on ruby 1.9.3.

    $ ruby -v
    ruby 1.9.3p194 (2012-04-20 revision 35410) [x86_64-darwin11.3.0]
    $ pry
    main> 'FooBar'.split(/(?=[A-Z])/).map{|_| _.downcase}.join('_')
    => "foo_bar"
    main> 'FooBar'.split(/(?=[A-Z])/)
    => ["Foo", "Bar"]

And on a recentish (pulled today) rbx, the downcase on Nil is pointless.

    $ ruby -v
    rubinius 2.0.0dev (1.9.3 e817532e yyyy-mm-dd JI) [x86_64-apple-darwin11.3.0]
    $ rbx -S pry
    main> 'FooBar'.split(/(?=[A-Z])/).map{|_| _.downcase}.join('_')
    NoMethodError: undefined method `downcase' on nil:NilClass.
    from kernel/delta/kernel.rb:81:in `downcase (method_missing)'
    main> 'FooBar'.split(/(?=[A-Z])/)
    => ["Foo", "", "", "Bar"]

Jruby 1.6.7 appears to behave as ruby 1.9.3.

@mitchty
Copy link
Author

mitchty commented May 1, 2012

Actually looking at this after i've had sleep it looks like I accidentally stumbled on two separate issues, one is the split with the regex the second is _ in map.

$ rbx -S pry
main> ["Foo", "", "", "Bar"].map {|_| _.downcase}.join('|')
NoMethodError: undefined method `downcase' on nil:NilClass.
from kernel/delta/kernel.rb:81:in `downcase (method_missing)'
main> ["Foo", "", "", "Bar"].map {|x| x.downcase}.join('|')
=> "foo|||bar"

Should I open a separate issue for the _ in map context or keep it in this issue? To be honest I rarely use _ like this but just happened to type it in pry this one time. 1.9.3-p194 behaves as expected.

@kronos
Copy link
Member

kronos commented May 1, 2012

I've written a patch that seems to fix a problem with split https://gist.github.com/81e25b635f26a1cb85d2

@ileitch
Copy link
Member

ileitch commented May 1, 2012

@mitchty yes, please open a new issue for the _ issue.
@kronos does this fix an existing spec? If not, you'll need to add a spec along with your patch also.

IPGlider added a commit that referenced this issue May 1, 2012
IPGlider added a commit that referenced this issue May 1, 2012
@IPGlider
Copy link
Member

IPGlider commented May 1, 2012

I modified the spec the other day while working on this issue, but did not commit it. Seems that @kronos patch fix the problem.

@kronos kronos closed this as completed in 0016d9c May 1, 2012
@kronos
Copy link
Member

kronos commented May 1, 2012

Problem with "_" isn't fixed but I think it's another issue

@dbussink
Copy link
Contributor

dbussink commented May 2, 2012

@kronos The issue with _ is indeed another issue, about to push a fix for that.

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

No branches or pull requests

5 participants