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

edit command does not work with Panic Nova #726

Closed
mzagaja opened this issue Jul 21, 2022 · 8 comments
Closed

edit command does not work with Panic Nova #726

mzagaja opened this issue Jul 21, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@mzagaja
Copy link
Contributor

mzagaja commented Jul 21, 2022

Your environment

mzagaja@Matthews-MacBook-Pro-16 ~/D/roadrunner (PLAT-1878-remove-unused-topics-routes) [SIGINT]> ruby -v
ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [x86_64-darwin21]
mzagaja@Matthews-MacBook-Pro-16 ~/D/roadrunner (PLAT-1878-remove-unused-topics-routes)> rdbg -v
rdbg 1.5.0

Describe the bug
When I type edit into Ruby debug, it outputs the command information, but does not actually open the editor with the file.

To Reproduce

bundle exec rspec spec/requests/follows_spec.rb
Symfony User is deprecated!  Switch to Identity::User.
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 59410
.......[9, 18] in ~/Developer/roadrunner/spec/requests/follows_spec.rb
     9|   let(:session_user) { new_logged_in_session_user(identity_user: identity_user) }
    10|
    11|   describe 'creating' do
    12|     context 'logged in' do
    13|       it 'should succesfully create a follow for a user' do
=>  14|         debugger
    15|         allow_any_instance_of(Identity::Follow).to receive(:save) { true }
    16|         sign_in session_user
    17|         post '/follows.json', params: { follow: { subject_type: 'User', subject_id: 1234 } }
    18|         expect(response.status).to eq(200)
=>#0	block in <top (required)> (4 levels) at ~/Developer/roadrunner/spec/requests/follows_spec.rb:14
  #1	[C] BasicObject#instance_exec at ~/.rvm/gems/ruby-2.7.6@roadrunner/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:263
  # and 48 frames (use `bt' command for all frames)
(rdbg) edit    # command
command: nova -w
   path: /Users/mzagaja/Developer/roadrunner/spec/requests/follows_spec.rb
(rdbg) edit    # command
command: nova -w
   path: /Users/mzagaja/Developer/roadrunner/spec/requests/follows_spec.rb
(rdbg) edit    # command
command: nova -w
   path: /Users/mzagaja/Developer/roadrunner/spec/requests/follows_spec.rb
(rdbg) edit    # command
command: nova -w
   path: /Users/mzagaja/Developer/roadrunner/spec/requests/follows_spec.rb
(rdbg) exit

Expected behavior
I expected the file the debugger is open in to open in Panic Nova.

@ko1
Copy link
Collaborator

ko1 commented Jul 27, 2022

https://github.com/ruby/debug/blob/master/lib/debug/thread_client.rb#L646

It only calls system(...). Do you have any idea?

@ko1
Copy link
Collaborator

ko1 commented Sep 16, 2022

Please reopen it if you find another ideas.

@ko1 ko1 closed this as completed Sep 16, 2022
@mzagaja
Copy link
Contributor Author

mzagaja commented Sep 26, 2022

@ko1 Thanks for your response, I have figured out the issue. I have set $EDITOR to nova -w because Panic Nova does not wait by default. This means calling system('nova', FILE_PATH) will continue execution before any changes are made.

The problem is calling system('nova -w', FILE_PATH) appears to not be valid because with the second argument system treats nova -w as the command instead of nova with the argument -w. Therefore working commands would be:

system('nova', '-w', FILE_PATH)
system("nova -w #{FILE_PATH}")

I am happy to make a pull request to suggest this change to the debug source you mention if it makes sense to you. Othewrise if this behavior seems wrong by Panic Nova, I can file a support ticket with them, they are usually responsive.

@ko1
Copy link
Collaborator

ko1 commented Oct 4, 2022

Ah I see. It is a bug of debug.gem.
I'll try it later (not sure when I can fix it) or patch is welcome.

@ko1 ko1 reopened this Oct 4, 2022
@ko1 ko1 added the bug Something isn't working label Oct 4, 2022
@ko1 ko1 added this to the v1.7.0 milestone Oct 4, 2022
@adam12
Copy link
Contributor

adam12 commented Oct 4, 2022

Maybe we should Shellwords.split before calling system?

diff --git a/lib/debug/thread_client.rb b/lib/debug/thread_client.rb
index 5f2d100..858fc0d 100644
--- a/lib/debug/thread_client.rb
+++ b/lib/debug/thread_client.rb
@@ -2,6 +2,7 @@

 require 'objspace'
 require 'pp'
+require 'shellwords'

 require_relative 'color'

@@ -643,7 +644,7 @@ module DEBUGGER__
         if editor = (ENV['RUBY_DEBUG_EDITOR'] || ENV['EDITOR'])
           puts "command: #{editor}"
           puts "   path: #{path}"
-          system(editor, path)
+          system(*Shellwords.split(editor), path)
         else
           puts "can not find editor setting: ENV['RUBY_DEBUG_EDITOR'] or ENV['EDITOR']"
         end

@mzagaja
Copy link
Contributor Author

mzagaja commented Oct 18, 2022

@adam12 That seems reasonable to me. Would be happy to see this included in next release. I'm happy to PR your diff if you are too busy but figured you might want the credit since you've pretty much done the work here 😄.

@adam12
Copy link
Contributor

adam12 commented Oct 18, 2022

@mzagaja If it the patch works for you, definitely go ahead and submit a PR using it :)

mzagaja added a commit to mzagaja/debug that referenced this issue Oct 21, 2022
When typing “edit” into the debugger, Panic Nova would not wait because it requires the -w flag, and the call to system was not respecting it. This commit uses the Shellwords.split method to separate it into another argument for system so Nova or other editors that require a flag work properly.
@mzagaja
Copy link
Contributor Author

mzagaja commented Oct 21, 2022

I tested this locally and it worked great. PR submitted.

@ko1 ko1 closed this as completed in e0633df Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

3 participants