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

Multiple class updates hide earlier method definition for show-method #934

Closed
tfuto opened this issue Jun 4, 2013 · 10 comments
Closed

Multiple class updates hide earlier method definition for show-method #934

tfuto opened this issue Jun 4, 2013 · 10 comments

Comments

@tfuto
Copy link

tfuto commented Jun 4, 2013

Adding a method to a class, then separately adding another method to the class will somehow hide the earlier method, and show-method just shows the more recent one. The methods are still there though

To reproduce:

[1] pry(main)> class A; def m1; end; end     
=> nil

[2] pry(main)> class A; def m2; end; end
=> nil

[3] pry(main)> show-method A#m1
[...]
class A; def m1; end; end

[4] pry(main)> show-method A#m2
[...]
class A; def m2; end; end

however:

[5] pry(main)> show-method A
[...]
class A; def m2; end; end

but:

[6] pry(main)> show-method A.new.m1
[...]
class A; def m1; end; end

[7] pry(main)> show-method A.new.m2
[...]
class A; def m2; end; end
@banister
Copy link
Member

banister commented Jun 4, 2013

ok, there's a few things going on here:

  1. Pry assumes that methods appear on a line of their own (this is a reasonable assumption for the most part). This means that when you show-method, it'll show the entire content of the line where the method was defined. In your examples, this will result in the class A; .. being displayed -- as this code appeared on the same line as the method definitions.
  2. From above, this explains why the output for show-method A#m1 and show-method A#m2 are what they are --- Pry is not attempting to show any kind of complete class definition, it's simply showing the lines of code used to define the method. No methods are being hidden here, furthermore you asked for the definitions of the methods not the class itself ;)
  3. when you show-source A (i prefer to use show-source than show-method as it can operate on more than just methods) -- Pry is simply showing one of the monkeypatches of the A class. Remember, in Ruby a class can be reopened multiple times in multiple files -- there is no single canonical class definition, and somehow 'merging' each of the individual monkeypatches into a single class definition would be extremely artificial, complicated and error prone. Instead Pry just shows the 'primary' monkeypatch (the one with the largest number of method definitions) by default -- and all other ones are available by passing the -a (for "all") switch to show-source.
  4. However, the show-source -a switch only works if the monkeypatches are defined on different files on disk -- it will not yet work on multiple monkeypatches defined in the REPL -- this is due to some complexities in implementation that i'll eventually get around to fixing ;)

@tfuto
Copy link
Author

tfuto commented Jun 4, 2013

@banister : great info! Thank you very much!! 1,2: got it. Re:3,4: actually for my simple test it worked, if I reordered the class definitions:

[1] pry(main)> class A
[1] pry(main)*   def m1
[1] pry(main)*   end  
[1] pry(main)* end  
=> nil
[2] pry(main)> class A
[2] pry(main)*   def m2
[2] pry(main)*   end  
[2] pry(main)* end  
=> nil
[3] pry(main)> class A
[3] pry(main)*   def m3
[3] pry(main)*   end  
[3] pry(main)* end  
=> nil
[4] pry(main)> show-source A

From: (pry) @ line 1:
Class name: A
Number of lines: 12

class A
  def m1
  end
end
class A
  def m2
  end
end
class A
  def m3
  end
end

Is this just coincidental? Was I just lucky then?

@banister
Copy link
Member

banister commented Jun 4, 2013

heheh coincidental -- Pry makes the assumption (again, assumptions make life easier, and if they're reasonable in 90% of cases i'm cool with it 👍) that there is only one monkeypatch of a given class per file -- the REPL is considered by Pry to be a kind of file (a sort of virtual file called "(pry)"). As a result of this assumption Pry does two things to extract a class definition:

  1. Find the first method defined in that file (i.e the file with the smallest source_location line number) and use this as a starting point for searching up and finding the line with class Foo on it.
  2. Find the last method defined in that file ... Pry uses this as a jump point, i.e -- once it's found the line with "class Foo" on it, it then immediately gobbles up all the intervening code between the "class Foo" line and the last method's line and then continues gobbling up lines until it gets a complete expression (this should be the end of the class definition).

In your example session, this is exactly what's happening, Pry will grab all the code between the first class A (associated with the m1 method) and the end of the final class where the last method was defined me.

This is fine in your case, but imagine if you had intervening code unrelated to the A class interspersed between the class definitions -- it kinda breaks, behold:

[1] pry(main)> class A                                                                                                                    
             |   def m1; end                                                                                                              
             | end                                                                                                                        
=> nil
[2] pry(main)> puts "this is random text that should not appear in output of show-source A" ;                                              
=> nil
[3] pry(main)> class A                                                                                                                    
             |   def m2; end                                                                                                              
             | end                                                                                                                        
=> nil
[4] pry(main)> puts "more random text";                                                                                                    
=> nil
[5] pry(main)> class A                                                                                                                    
             |   def m3; end                                                                                                              
             | end                                                                                                                        
=> nil
[6] pry(main)> $ A                                                                                                                        

From: (pry) @ line 1:
Class name: A
Number of lines: 11

class A
  def m1; end
end
puts "this is random text that should not appear in output of show-source A";
class A
  def m2; end
end
puts "more random text";
class A
  def m3; end
end
[7] pry(main)>                                                                                                                            

@tfuto
Copy link
Author

tfuto commented Jun 4, 2013

Okay, I got it. I did an extended test, and I understand your comments better. Well, it is not faultless, but definitely not useless either.

Created two files:

~/test$ cat a.rb
class A
  def m1
    '1'
  end
end
class A
  def m2
  end
end

~/test$ cat b.rb
class A
  def m1
    '2'
  end
end
class A
  def m3
  end
end

Fire up pry and load them:

~/test$ pry
[1] pry(main)> load 'a.rb'
=> true
[2] pry(main)> load 'b.rb'
=> true

m1 is redefined:

[3] pry(main)> class A
[3] pry(main)*   def m1
[3] pry(main)*     '3'
[3] pry(main)*   end  
[3] pry(main)*   def m4
[3] pry(main)*   end  
[3] pry(main)* end  
=> nil
[4] pry(main)> show-source A

From: (pry) @ line 3:
Class name: A
Number of monkeypatches: 3. Use the `-a` option to display all available monkeypatches
Number of lines: 7

class A
  def m1
    '3'
  end
  def m4
  end
end
[5] pry(main)> show-source -a A
Found 3 candidates for `A` definition:

Candidate 1/3: (pry) @ line 3:
Number of lines: 7

class A
  def m1
    '3'
  end
  def m4
  end
end

Candidate 2/3: a.rb @ line 6:
Number of lines: 4

class A
  def m2
  end
end

Candidate 3/3: b.rb @ line 6:
Number of lines: 4

class A
  def m3
  end
end

Looks good. However, if I redefine m1 again:

[7] pry(main)> class A
[7] pry(main)*   def m1
[7] pry(main)*     '4'
[7] pry(main)*   end  
[7] pry(main)* end  
=> nil
[8] pry(main)> show-source -a A
Found 3 candidates for `A` definition:

Candidate 1/3: (pry) @ line 3:
Number of lines: 12

The duplication of m1 is an issue here:

class A
  def m1
    '3'
  end
  def m4
  end
end
class A
  def m1
    '4'
  end
end

Candidate 2/3: a.rb @ line 6:
Number of lines: 4

class A
  def m2
  end
end

Candidate 3/3: b.rb @ line 6:
Number of lines: 4

class A
  def m3
  end
end
[9] pry(main)> show-source A

From: (pry) @ line 3:
Class name: A
Number of monkeypatches: 3. Use the `-a` option to display all available monkeypatches
Number of lines: 12

Also, m1 is duplicated in the list without -a:

class A
  def m1
    '3'
  end
  def m4
  end
end    
class A
  def m1
    '4'
  end
end

@tfuto
Copy link
Author

tfuto commented Jun 4, 2013

@banister : Thank you again for all the information!

@banister
Copy link
Member

banister commented Jun 4, 2013

Yeah, I can probably prevent the display of multiple definitions in the REPL, but outside of the REPL and considering class definitions from multiple files it would truly be a messy process to get rid of the duplicate method defintions. Assume i had two class definitions in two separate files:

a.rb

class A
  def m1; end
  def m2; end
end

b.rb

class A
   def m1;end
   def m3;end
end

If i show-source -a A on that i will get BOTH class definitions including the multiple definitions of m1 -- but how would i get rid of that? I would have to somehow reconstruct class definitions looking only at the list of 'existing' methods, this is a complete pain in the ass, cos i'd also have to take into account things like the visibility of the methods, whether methods appear in class << self blocks, and what about constants? If i'm trying to artificially construct a "canonical class definition" i'd also have to merge the constants from all the various classes, putting them in the correct place. Not only is this process incredibly messy and error prone, it's also totally artificial as this "canonical class definition" exists nowhere on disk.

Instead of that messy process, i opted to instead extract actual class definitions as they appear on disk, that is, if there's a file somewhere that contains code with class A ... in it, i will show the content of that code -- irrespective of whether another file somewhere else also has class A ... which overrides some of the same methods. This approach is realistic, as the code actually appears on disk, and simple.

I agree however there might be some confusion as to which methods are active and which are not when viewing multiple class definitions that override the same of methods -- in future i could perhaps 'tag' the methods that are inactive with an asterix or some such, to indicate they've been overriden.

@tfuto
Copy link
Author

tfuto commented Jun 4, 2013

Well, it is not easy, I can see that now... Especially aliases makes this pretty insane. :-) Okay, so I understand the intended use and usefulness, and if you have time, please support the REPL multi-patch issue. I do not know if you want me to close this issue or keep it open. Thanks again!

@banister
Copy link
Member

banister commented Jun 5, 2013

I think you want the Pry edit command, see the following showterm: http://showterm.io/434d8965312292c8830c9#fast

@tfuto
Copy link
Author

tfuto commented Jun 5, 2013

Ok, I understand your flow, and it makes sense to follow it. Thank you!

@tfuto tfuto closed this as completed Jun 5, 2013
@banister
Copy link
Member

banister commented Jun 5, 2013

@tfuto yes, you'll actually find using the edit command a lot more convenient - when you type in large chunks of code in the REPL (like class definitions) it can be a pain when you make a mistake (you have to start over). The edit command automatically reloads files, and it allows you to target methods or classes (or files if u prefer). Let me know if this solution is fine for you or whether you encounter any problems.

rf- added a commit that referenced this issue May 3, 2014
This is based on this commit:

  cosmo0920/win32-api@ceb47f0eb48e

It should fix the issue where we were only building i386-specific gems
for Windows.
ConradIrwin added a commit that referenced this issue May 15, 2014
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

2 participants