[ruby-core:125265] [Ruby Feature#21998] Add {Method,UnboundMethod,Proc}#source_range
Issue #21998 has been reported by Eregon (Benoit Daloze). ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998 * Author: Eregon (Benoit Daloze) * Status: Open ---------------------------------------- I'm using matz's suggestion almost as-is from https://bugs.ruby-lang.org/issues/6012#note-53. The only change is the proposed class name. ## Use Cases Use cases have been discussed extensively and matz said:
The use cases are real and I want to support them
So I think we don't need to discuss that anymore :) ## Background Adding column and last line information to `#source_location` is deemed too incompatible given the usages of `obj.source_location.last` which expect the start line (they would get the end column instead). ## Proposal So instead we add a new method, `{Method,UnboundMethod,Proc}#source_range`, which returns a `Ruby::SourceRange` and has these methods: * `start_line`: 1-indexed (same as `source_location.first`) * `end_line`: 1-indexed * `start_column`: in bytes, 0-indexed, I think `start_byte_column` could be good for extra clarity * `end_column`: in bytes, 0-indexed, I think `end_byte_column` could be good for extra clarity * `inspect` which shows something like `#<Ruby::SourceRange (1,0)-(2,10)>`. For the edge case of a heredoc spanning further than the end of a method/block, we would not include it in the `end_line` & `end_column` methods, but instead document it and provide a small code snippet in the docs to compute that if desired with Prism. ## Alternative An alternative could be a `Ruby::SourceLocation` class and `obj.source_location(object: true)`/`obj.source_location(extended: true)` but it feels less nice. Since we are designing something new I think it's best to go for the cleanest design. ## Consistency The above methods match the name of methods on `Prism::Node` and have the same semantics, which is good for consistency and to avoid confusion. ## Scope The scope is intentionally minimal to keep the discussion focused. ## Implementation I'm happy to implement this, it should be pretty trivial and very similar to the extended `source_location`. I would prefer to get an approval for this feature before implementing. -- https://bugs.ruby-lang.org/
Issue #21998 has been updated by matz (Yukihiro Matsumoto). Thank you for the proposal. Introducing a new method rather than extending `source_location` is the right direction, given the compatibility issues we hit. I approve the shape of this feature, including: - The class name `Ruby::SourceRange` (matches the method name). - The four accessors `start_line`, `end_line`, `start_column`, `end_column`. - Byte-based columns, as long as this is clearly documented. - Excluding heredocs from `end_line`/`end_column` with documentation of the workaround. - Not adding `Binding#source_range`. Matz. ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117058 * Author: Eregon (Benoit Daloze) * Status: Open ---------------------------------------- I'm using matz's suggestion almost as-is from https://bugs.ruby-lang.org/issues/6012#note-53. The only change is the proposed class name. ## Use Cases Use cases have been discussed extensively and matz said:
The use cases are real and I want to support them
So I think we don't need to discuss that anymore :) ## Background Adding column and last line information to `#source_location` is deemed too incompatible given the usages of `obj.source_location.last` which expect the start line (they would get the end column instead). ## Proposal So instead we add a new method, `{Method,UnboundMethod,Proc}#source_range`, which returns a `Ruby::SourceRange` and has these methods: * `start_line`: 1-indexed (same as `source_location.first`) * `end_line`: 1-indexed * `start_column`: in bytes, 0-indexed, I think `start_byte_column` could be good for extra clarity * `end_column`: in bytes, 0-indexed, I think `end_byte_column` could be good for extra clarity * `inspect` which shows something like `#<Ruby::SourceRange (1,0)-(2,10)>`. For the edge case of a heredoc spanning further than the end of a method/block, we would not include it in the `end_line` & `end_column` methods, but instead document it and provide a small code snippet in the docs to compute that if desired with Prism. ## Alternative An alternative could be a `Ruby::SourceLocation` class and `obj.source_location(object: true)`/`obj.source_location(extended: true)` but it feels less nice. Since we are designing something new I think it's best to go for the cleanest design. ## Consistency The above methods match the name of methods on `Prism::Node` and have the same semantics, which is good for consistency and to avoid confusion. ## Scope The scope is intentionally minimal to keep the discussion focused. ## Implementation I'm happy to implement this, it should be pretty trivial and very similar to the extended `source_location`. I would prefer to get an approval for this feature before implementing. -- https://bugs.ruby-lang.org/
Issue #21998 has been updated by mame (Yusuke Endoh). @matz What should the following return? ```ruby f = proc { <<END } xxx END f.source_range #=> Start position: from the `p` of `proc`? Or from the `{`? #=> End position: up to the `}`? Or up to the `D` of `END`? ``` I don't understand the use cases for this feature, so I'm not sure what it should return. A related feature that Eregon also mentioned in #6012, `show_source` in irb, behaves as follows, so perhaps the heredoc should be included? ``` $ irb -rmethod_source irb(main):001" def foo = (<<END) irb(main):002" xxx irb(main):003> END => :foo irb(main):004> show_source foo From: (irb):1 def foo = (<<END) xxx END ``` ---------------------------------------- Feature #21998: Add {Method,UnboundMethod,Proc}#source_range https://bugs.ruby-lang.org/issues/21998#change-117059 * Author: Eregon (Benoit Daloze) * Status: Open ---------------------------------------- I'm using matz's suggestion almost as-is from https://bugs.ruby-lang.org/issues/6012#note-53. The only change is the proposed class name. ## Use Cases Use cases have been discussed extensively and matz said:
The use cases are real and I want to support them
So I think we don't need to discuss that anymore :) ## Background Adding column and last line information to `#source_location` is deemed too incompatible given the usages of `obj.source_location.last` which expect the start line (they would get the end column instead). ## Proposal So instead we add a new method, `{Method,UnboundMethod,Proc}#source_range`, which returns a `Ruby::SourceRange` and has these methods: * `start_line`: 1-indexed (same as `source_location.first`) * `end_line`: 1-indexed * `start_column`: in bytes, 0-indexed, I think `start_byte_column` could be good for extra clarity * `end_column`: in bytes, 0-indexed, I think `end_byte_column` could be good for extra clarity * `inspect` which shows something like `#<Ruby::SourceRange (1,0)-(2,10)>`. For the edge case of a heredoc spanning further than the end of a method/block, we would not include it in the `end_line` & `end_column` methods, but instead document it and provide a small code snippet in the docs to compute that if desired with Prism. ## Alternative An alternative could be a `Ruby::SourceLocation` class and `obj.source_location(object: true)`/`obj.source_location(extended: true)` but it feels less nice. Since we are designing something new I think it's best to go for the cleanest design. ## Consistency The above methods match the name of methods on `Prism::Node` and have the same semantics, which is good for consistency and to avoid confusion. ## Scope The scope is intentionally minimal to keep the discussion focused. ## Implementation I'm happy to implement this, it should be pretty trivial and very similar to the extended `source_location`. I would prefer to get an approval for this feature before implementing. -- https://bugs.ruby-lang.org/
participants (3)
-
Eregon (Benoit Daloze) -
mame (Yusuke Endoh) -
matz (Yukihiro Matsumoto)