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

byebug improvements #263

Closed
ccoupe opened this issue Jul 18, 2016 · 16 comments
Closed

byebug improvements #263

ccoupe opened this issue Jul 18, 2016 · 16 comments
Labels
Milestone

Comments

@ccoupe
Copy link
Contributor

@ccoupe ccoupe commented Jul 18, 2016

Now that osx, linux and windows have working launch scripts and we can depend on the underlying native console, we should try again to get `-d script.rb' working. Note that would only work for console launch. This issue has nothing to do with Shoes.terminal (because that's never going to be a readline compatible thing.

cshoes -d [script.rb] and or ./dist/shoesor other command line invocations like cshoes.exe should work. First is to get the command line parsed. Then get byebug to white-list the GUI threads from being suspended. While remote debugger mostly works, it could be easier . Lastly 'debugging' from the splash screen doesn't actually work. Yet.

That thread white-list is dependent on byebug - it's a fairly active project with responsive maintainers so I'm sure we can get this figured out.

@ccoupe ccoupe added the Normal label Jul 18, 2016
@ccoupe ccoupe added this to the 3.3.2 milestone Jul 18, 2016
ccoupe added a commit that referenced this issue Jul 18, 2016
ccoupe added a commit that referenced this issue Jul 20, 2016
* all fail stepping into/next/continue-to-brk-pt in  Shoes.app
@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 20, 2016

After looking at the byebug 8.2.2 code - things are broken for -d and what I remember about 5.0.2 internals doesn't help anyone since it's been simplified. here's crappy test script:

# some tests of debugging
# next over: 
one = 1
two = "two"

Shoes.app do
  stack do
    para one
    para two
    flow do
      button "rdb" do
        require 'byebug'
        byebug
        para "rdb: #{one} #{two}"
      end
    end
  end
end

using -d and terminal launch of shoes/cshoes you can step through ruby upto the Shoes.app.
When Terminal launching the app w/o -d and then clicking the 'rdb' button gets a byebug loop running on the lauch terminal. Remote debug works. One other confounder: if you add

 # next over: 
require 'byebug'
byebug
one = 1

and terminal launch w/o -d then byebug can step into shoes.app , set break points and c' to them Of course the whole point of-d` is to not modify the script. I'm nowhere near there and I'm feeling in need of help.

@ccoupe ccoupe added the Low label Jul 20, 2016
@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 20, 2016

Set priority to low. Nobody is using it or they would have complained,

@dredknight
Copy link
Contributor

@dredknight dredknight commented Jul 21, 2016

I didnt know Shoes had debuger. I think it will be very nice to use !

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 21, 2016

@dredknight , the debugger can be confusing and illuminating in Shoes because of those blocks and how Shoes execute things. If it's mostly just Ruby, it works pretty well. There is a lot of power in byebug but it takes some learning and thinking which I'm just starting to acquire.

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 21, 2016

Updated the wiki a bit, as I really learn what works.

ccoupe added a commit that referenced this issue Jul 23, 2016
* -d is going away since it's trivial for the user to enable
  their script for debugging with their own command line parsing.
  see
@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 23, 2016

Something I've wished for since I first started working on Shoes. Debugging Shoes internals in Ruby: Currently on branch 'sdb'. OSX might be a problem but this is pretty secret voodoo that only insiders care about.

ccoupe@bronco:~/Projects/shoes3$ dist/shoes --sdb 

[6, 15] in /home/ccoupe/Projects/shoes3/lib/shoes.rb
    6: #
    7: if SHOES_DEBUG
    8:   require 'byebug'
    9:   byebug
   10: end
=> 11: require_relative 'shoes/cache' # do First thing
   12: ARGV.delete_if { |x| x =~ /-psn_/ }
   13: 
   14: # Probably don't need this
   15: class Encoding
(byebug) b /home/ccoupe/Projects/shoes3/lib/shoes/cache.rb:1
Successfully created breakpoint with id 1
(byebug) c
Stopped by breakpoint 1 at /home/ccoupe/Projects/shoes3/lib/shoes/cache.rb:1

[1, 10] in /home/ccoupe/Projects/shoes3/lib/shoes/cache.rb
=>  1: require 'fileutils'
    2: include FileUtils
    3: # add it download.rb monkey patches Shoes download -replaces curl
    4: require_relative 'download.rb'
    5: # locate ~/.shoes
    6: require 'tmpdir'
    7: lib_dir = nil
    8: homes = []
    9: homes << [ENV['LOCALAPPDATA'], File.join( ENV['LOCALAPPDATA'], 'Shoes')] if ENV['LOCALAPPDATA']
   10: homes << [ENV['APPDATA'], File.join( ENV['APPDATA'], 'Shoes' )] if ENV['APPDATA']
(byebug) b /home/ccoupe/Projects/shoes3/lib/shoes/cobbler.rb:238
Successfully created breakpoint with id 2
(byebug) c
Stopped by breakpoint 2 at /home/ccoupe/Projects/shoes3/lib/shoes/cobbler.rb:238

[233, 242] in /home/ccoupe/Projects/shoes3/lib/shoes/cobbler.rb
   233:       end
   234:     end
   235:   end
   236: 
   237:   def infoscreen
=> 238:     @panel.clear
   239:     @panel.append do
   240:       para "Ruby Version: #{RUBY_VERSION} on #{RUBY_PLATFORM}"
   241:       para "Shoes Release: #{Shoes::RELEASE_NAME}   #{Shoes::VERSION_NUMBER}  r#{Shoes::VERSION_REVISION}"
   242:       para "    built on #{Shoes::VERSION_DATE}"
(byebug) 
) 

You can set breakpoints on files not currently loaded (if you supply the right path) so --sdb it can be used for debugging user scripts too.

@ccoupe ccoupe removed the Low label Jul 23, 2016
ccoupe added a commit that referenced this issue Jul 23, 2016
* of course --sdb does what -d was intended to do. DUH!
  delete --sdb and use -d
ccoupe added a commit that referenced this issue Jul 25, 2016
* Duh! Can't require byebug until after shoes has run cache.rb
  Unless it's loose shoes and you have byebug installed - head fake.
ccoupe added a commit that referenced this issue Jul 26, 2016
* can grab files that are named the same. #262
* also true of @backorders irb.rb. Change the name
* #263 - now inside byebug one can invoke irb (for better or worse).
* irb complains that exit and quit can't overridden. Too true.
@passenger94
Copy link
Contributor

@passenger94 passenger94 commented Jul 26, 2016

@ccoupe for President !
Damn cool !!

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 27, 2016

One problem. irb tries to alias exit and Shoes is not co-operating. Perhaps quit too. Not sure how to fix that but until I do the only way to get out of byebug->irb is pkill shoes from another terminal.

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 27, 2016

That one problem is a big problem involving deep deep do-do about exit and quit. Been in there. don't want to be there. irb and pry commands should be disabled from byebug if in Shoes. Even when Shoes exit and quit are modified for irb, it does the wrong thing (for a Shoes scripter). Not worth fixing and I'm not sure it can be fixed.

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 28, 2016

Branch 'sdb':

When starting up normally -- SHOES_DEBUG == false, then exit and quit point to the Shoes redefined method - shoes_app_quit. When SHOES_DEBUG == true ( from command line -d ) then exit and quit do not point to Shoes. They point to base Ruby (or byebug invoked irb) definitions of quit and exit. This affects Shoes scripts being debugged if they call quit or exit -- they'll log an error to the Shoes console (aka log.rb). Use the window close widget.

This is not optimal. But, you can invoke irb from byebug if Shoes was started -d. I strongly suggest you do not use irb but you can get Shoes.APPS and probably get self setup so you could enter code from irb. byebug->irb is not available if you don't start from -d and invoke byebug in your script.

Still on branch sdb, so it's not final: I've implemented Shoes.quit() and Shoes.exit() methods to be used instead of Ruby quit() or exit(). This API change will break some existing scripts (bugs, samples, some of my own and.... ?). Closing from the window decorator still works.

Please discuss - @BackOrder, @passenger94 ? My thoughts: Aside from the byebug/irb mess Shoes should not redefine Ruby exit and quit. It was a clever hack back in the old times. But it is a hack and not every hack can be carried into the future. It's not like Ruby is standing still.

ccoupe added a commit that referenced this issue Jul 28, 2016
ccoupe added a commit that referenced this issue Jul 28, 2016
* backwards compatible.
* prepare for new API #263
@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 28, 2016

Branch master: Through the magic of Shoes, inheritance and duck typing we always could have called Shoes.quit and Shoes.exit. Who knew? This make the proposed API transition not so terrible.

@passenger94
Copy link
Contributor

@passenger94 passenger94 commented Jul 28, 2016

This means, no breakage ?
Because yes, superficially looking at the changes it sounded scary, to be honest !

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 29, 2016

No, there is still breakage. Byebug and irb really doesn't like how Shoes has defined exit and quit in Ruby kernel. It's easy enough to not create them when running -d. Not so easy (impossible?) if running remote byebug. If shoes doesn't override exit and quit when running -d then the only way to exit is the window close button or by calling Shoes.exit or Shoes.quit - which just happens to work already in Shoes 3.3.1 whether the sdb branch is merged to master or not.

In short, I'm proposing moving Shoes quit/exit from Ruby kernel space to Shoes.space. That breaks the API. Shoes.space has all kinds of oddities like fullscreen and terminal ... exit and quit fit there reasonably well. While it is clever to redefine Kernel and Object methods, it is a hack just waiting for a Ruby API change to break things - and they do change things down in the kernel. IMO, I think it is worth the user pain to fix any Shoes scripts that explicitly call exit or quit. It's not a lot of pain and users can start now.

FWIW, the quit method is not documented in the manual and the description of exit is worth reading. Now that I've some more time to ponder, I think Shoes.quit should be way to stop Shoes scripts.

I think irb is pretty useless in Shoes because it is confusing down there inside Shoes, but that's just me and I suspect some .irbrc settings /macros/new shoes apis might make it easier. I don't see pry being useful in a Shoes context but I could be wrong and I don't want to close the door on either of them if someone wants them enough to do that work.

@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 29, 2016

One side benefit to getting -d working: There is no good reason (IMO) to have remote debug which was damn clunky at it's best. There are still some puzzles with exit, should it be called from Shoes (branch sdb). Must think more.

ccoupe added a commit that referenced this issue Jul 31, 2016
* but they may not work if the debugger is active
@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jul 31, 2016

I think I've got a new position: exit and quit will work as always (backwards compatible) until you use byebug either from -d or invoking it in a script. Then quit and exit may not close the windows and exit the app. Use the window decorator. If you get stuck in irb without a working quit and exit, ^D will get you back to byebug. If your code needs the debugger and therefore the command line, you can handle some inconsistency in quit/exit behavior.

You can and should use Shoes.quit because it works in all those situations. FYI, in ruby.c where we do rb_define_method(rb_mKernel,.....);, I think those are actually made in the Object class and not in the Kernel module. One more reason to not use exit for Shoes programs.

ccoupe added a commit that referenced this issue Jul 31, 2016
@ccoupe ccoupe mentioned this issue Aug 9, 2016
ccoupe added a commit that referenced this issue Sep 26, 2016
* completly out out my depth. How to draw wedges? Anybody want to take
  that on. I don't
* So many chart things could be done but (and known bugs do exist) we have
  to think about "does anyone care?" And diminishing returns.
@ccoupe
Copy link
Contributor Author

@ccoupe ccoupe commented Jan 21, 2017

Closed in Shoes 3.3.2

@ccoupe ccoupe closed this Jan 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants