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

gsub_file with block, no access to $1 etc? #207

Open
jrochkind opened this issue Dec 28, 2011 · 12 comments
Open

gsub_file with block, no access to $1 etc? #207

jrochkind opened this issue Dec 28, 2011 · 12 comments

Comments

@jrochkind
Copy link

When using gsub_file action with a block, if the expression matched had paren grouping, it seems you ought to be able to access $1 etc in the block, to construct your replacement string.

Looking at the source code, it still seems like this should work -- the block passed in to gsub_file is passed to ordinary String#gsub!, which definitely makes $1 etc available in the block.

And yet, I can not get this to work. (0.14.6, in context of a Rails 3.1 generator). $1 is always nil, even if there was a matching paren group in the regexp.

Anyone have any idea what's going on?

@kentaroi
Copy link

As far as I know, we cannot access to $1, $&, Regexp.last_match etc in the block of any wrapping methods of String#gsub.

Actually, it was discussed by ruby core team here (, although it is in Japanese),
and they have not yet come to a decision about what's right for block parameter of gsub,
whether to change block parameter from String to MatchData or to change it to same as that of String#scan.

@jrochkind
Copy link
Author

Weird, special deal of $1 etc I guess. Thanks for confirming.

Problem is that this makes the block form of gsub_file not all that useful, yeah?

Yeah, if block form of gsub gave you a MatchData, that would certainly solve things. Hmm, I wonder, should thor gsub file use some other implementation, such that it can grab a MatchData and pass it to thor.gsub_file's own block, so the block can have access to the paren groups etc? Hmm, not sure there's any clean way to do that though, bah.

@kentaroi
Copy link

I'm not sure which way is the best.

Just an idea, but one of the ways is this:
kentaroi@46b6d0b
This change would make capturing groups accessible from a block of gsub_file, such as:

gsub_file(file_path, regexp) { |*match| match[2].upcase }
gsub_file(file_path, regexp) { |m0, m1, m2| m2.upcase }

the above two lines will get same results.

Also you can use the old style with a single block parameter, such as:

gsub_file(file_path, regexp) { |match| match.gsub(/__/, '').upcase }

However, the following are still not accessible:

  1. named capturing groups
  2. pre_match
  3. post_match

What do you think?

@jrochkind
Copy link
Author

I like it!

Weird that the block has no access to $1 etc, but somehow still has access to $& and $~ (neither of which I knew existed).

Is it backwards compat with old use? Appears so as you didn't remove/change any existing tests?

Neither of us are committers on thor though, huh? Wonder how we get the attention of someone who is, on this proj that doesn't have too much maintainer attention at the moment.

You're not a committer on

@jrochkind
Copy link
Author

Ah yes, I see $& is the backwards-compat one.

And can I use $~ in my block myself manually in existing-released thor? That's useful info to allow me to do what I need to do (hackily) already, awesome. Very odd that ruby will let the block see $~ but not $1, $2, etc, huh?

@kentaroi
Copy link

I am not good at English, so I might miss the point.

You can access to $, $&, $1, etc in the block of String#gsub!, but if you wrap the String#gsub! method and you call the wrapping method with block, you cannot access them ($, $&, $1) from the latter (wrapping method's) block.

Because, I think, a block drag the scope where the block was written.
The wrapping method's block has no access to $, $&, $1 in wrapping method but has access to $, $&, $1 where the block was written.

Ruby core team discussed current block parameter of gsub! is not so useful and we sometimes have to use global variables such as $1 etc, and should change block parameter of gsub! to more useful one. But, the change of block parameter is definetely backwards incompatible and it is still pending. Maybe, in the future, the block parameter of gsub! might be changed.

So the situation is we can write wrapping methods of String#gsub! which passes MatchData as a block parameter.
Current block parameter of Actions#gsub_file is same as that of String#gsub!. I think it is natural.
But we want to access to MatchData.

I wonder which is the best block parameter of Actions#gsub_file

  1. Just passes $~ ( backwards incompatible, no way.)
  2. Passes *$~ (backwards compatible, but have not access to named capturing groups etc. I wrote a draft of this in the above)
  3. Passes $~ as second block parameter (pragmatic but might not be beautiful)

I am not a committer but I want to send a pull request if I see the best block parameter.

@jrochkind
Copy link
Author

Ah, I understand, thanks!

I think this would be a good change to thor, personally. I think your
backward compatible idea of *$~ is good. Alternately, a non-backward
compatible version with a different method name could be provided with
$~

Maybe we can attract the attention of a committer.

On Jan 29, 2012, at 5:10 PM, Kentaro Imai wrote:

I am not good at English, so I might miss the point.

You can access to $, $&, $1, etc in the block of String#gsub!, but
if you wrap the String#gsub! method and you call the wrapping method
with block, you cannot access them ($
, $&, $1) from the latter
(wrapping method's) block.

Because, I think, a block drag the scope where the block was written.
The wrapping method's block has no access to $, $&, $1 in wrapping
method but has access to $
, $&, $1 where block was written.

Ruby core team discussed current block parameter of gsub! is not so
useful and we sometimes have to use global variables such as $1 etc,
and should change block parameter of gsub! to more useful one. But,
the change of block parameter is definetely backwards incompatible
and it is still pending. Maybe, in the future, the block parameter
of gsub! might change.

So the situation is we can write wrapping methods of String#gsub!
which passes MatchData as a block parameter.
Current Actions#gsub_file 's block parameter is same as
String#gsub!. I think it is natural.
But we want to access to MatchData.

I wonder which block parameter of Actions#gsub_file is the best

  1. Just passes $~ ( backwards incompatible, no way.)
  2. Passes *$~ (backwards compatible, but have not access to named
    capturing groups etc. I wrote a draft of this in the above)
  3. Passes $~ as second block parameter (pragmatic but might not be
    beautiful)

I am not a committer, so...


Reply to this email directly or view it on GitHub:
#207 (comment)

@jrochkind
Copy link
Author

Weird, in 1.8.7 it works fine, no? Wonder why ruby core team doesn't just make it work like it did in 1.8.7

@kentaroi
Copy link

kentaroi commented Feb 2, 2012

OK. I'll send a pull request.
Do you have any suggestions about the above code?

@kentaroi
Copy link

kentaroi commented Feb 5, 2012

See pull-req #217

@jherdman
Copy link
Contributor

This is definitely broken as near as I can tell.

@bryanbraun
Copy link

Yeah, this would still be useful. Any reason why it never made it in? It looks like the pull request was closed without any comment.

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

4 participants