Project

General

Profile

Feature #15419

Allow Kernel#tap to be invoked with arguments like Kernel#send

Added by shuber (Sean Huber) 4 months ago. Updated 4 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:90553]

Description

Tapping methods without any arguments already has nice shorthand via Symbol#to_proc:

object.tap { |o| o.example }
# vs
object.tap(&:example)

Unfortunately once other arguments are involved we have to switch back to the longer form:

array.merge(other).tap { |a| a.delete(object) }

This patch introduces a convenient and familiar shorthand for these cases which behaves similar to Kernel#send:

array.merge(other).tap(:delete, object)

Calling tap without any arguments or block still raises LocalJumpError:

3.tap #=> LocalJumpError: no block given

This also makes the existing shorthand even shorter:

object.tap { |o| o.example }
# vs
object.tap(&:example)
# vs
object.tap(:example)

Pull request: https://github.com/ruby/ruby/pull/2050

History

#1

Updated by shuber (Sean Huber) 4 months ago

  • Description updated (diff)
#2

Updated by shuber (Sean Huber) 4 months ago

  • Description updated (diff)
#3

Updated by shuber (Sean Huber) 4 months ago

  • Description updated (diff)

Updated by shuber (Sean Huber) 4 months ago

The ruby/spec on Ruby 2.3 check is failing in this PR - I'm not sure what the existing conventions are for adding specs for new features like this.

  • Should the new tests be wrapped in unless RUBY_VERSION <= "2.3" conditions?
  • Or should the new tests be added somewhere other than /spec instead?
  • Or some other convention? Please advise!

Updated by Eregon (Benoit Daloze) 4 months ago

shuber (Sean Huber) wrote:

The ruby/spec on Ruby 2.3 check is failing in this PR - I'm not sure what the existing conventions are for adding specs for new features like this.

  • Should the new tests be wrapped in unless RUBY_VERSION <= "2.3" conditions?
  • Or should the new tests be added somewhere other than /spec instead?
  • Or some other convention? Please advise!

Yes, in such a case you need to use ruby_version_is guards as documented on https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#guards,
because the behavior of previous versions will not change (unless this is decided to be backported, but very unlikely for a new feature, and even then the backport would be done later so the guards would be needed at first).

Updated by oleynikov (Alexander Oleynikov) 4 months ago

It is not backward compatible.
And I'm not sure why we need to change just #tap and not other methods.

There are some discussions going on about passing arguments whith Symbol#to_proc shorthand: #12115, #15301, etc. Personally I'd rather see one of these implemented. It would also fix this issue.

Updated by shuber (Sean Huber) 4 months ago

Eregon (Benoit Daloze) wrote:
Yes, in such a case you need to use ruby_version_is guards as documented on https://github.com/ruby/spec/blob/master/CONTRIBUTING.md#guards,
because the behavior of previous versions will not change (unless this is decided to be backported, but very unlikely for a new feature, and even then the backport would be done later so the guards would be needed at first).

Perfect thanks Eregon (Benoit Daloze), pull request updated!

oleynikov (Alexander Oleynikov) wrote:
And I'm not sure why we need to change just #tap and not other methods.
There are some discussions going on about passing arguments whith Symbol#to_proc shorthand: #12115, #15301, etc. Personally I'd rather see one of these implemented. It would also fix this issue.

Thanks for the links to the other discussions oleynikov (Alexander Oleynikov), pretty interesting, I'll keep an eye on those as well!

For Kernel#tap specifically I think it still makes sense to allow it to accept arguments like Kernel#send since they basically behave the same aside from tap always returning self. Relying on the proposals to support Symbol#to_proc with arguments (which could take awhile until they're actually merged) looks a bit more abstract than it needs to be:

array.tap(:delete, object)
# vs
array.tap(&:delete.(object)) # if that's the syntax they agree on
# vs
array.tap { |a| a.delete(object) } # currently supported

I could get behind something like this too:

array.tap(&:delete, object)

Also available in: Atom PDF