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

Update README.md to show multiple commands execution using binding.b #828

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

binarygit
Copy link
Contributor

@binarygit binarygit commented Nov 23, 2022

Description

Describe your changes:
This addition to the README.md demonstrates how to specify multiple commands to run when using binding.b.

I didn't think it obvious that you needed to have two semicolons separating the commands (cmd 1 ;; cmd 2). The README does not document this which is why I thought I would add it there.

I've also added an example to demonstrate how you could use pre and do arguments together to create a more advanced/useful workflow.

P.S. I wouldn't have known how to do both of the above things if it hadn't been for @st0012 Ruby Kaigi talk on debugging. So Thank You for that!

@ko1
Copy link
Collaborator

ko1 commented Nov 24, 2022

;; is temporary syntax and I'm not sure we should continue to maintain it.
This is why I didn't documented it.

For the pre:, do: command, I think "\n" (newline) is better.

@@ -586,6 +586,22 @@ end

On this case, you can see the result of `bar()` every time you stop there.

You can use `pre` and `do` together to create more useful workflows.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this added block is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't very obvious to me, how I would achieve something like this. I think the technique used is handy in alot of situations but as I said, it wasn't obvious how I would go about doing it. So, I think it's useful to have this documented here.

I would have never thought that

binding.b do: 'watch @a ;; break foo pre: info'

the pre inside the string above would be evaluated correctly, like an argument to a binding.b and thus suspend the program. I would have never thought this would work, so I think it's worth documenting.

But If I missed something in the README that does suggest that this is possible and I missed it, please let me know 😄

Copy link
Collaborator

@ko1 ko1 Nov 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why it is not clear without explanation. do: accepts debug command string (described) and break foo pre: info is debug command.

BTW, on this case,

debugger do: 'watch @a'
debugger do: 'break foo pre: info'

# or
debugger do: <<~DO
  watch @a
  break foo pre: info
DO

is more readable for me.
(I usually use ;; for writability/easy to write, though)

Copy link
Contributor Author

@binarygit binarygit Nov 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! I went through the docs again and see clearly that you have it there.
Sorry, I should have been more thorough!

I will remove the block from my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated my PR removing the above block and I added a sentence about how users can use ;; instead of \n.
I think this will do, no?
Let me know if you want me to add something else.

Also hope ;; stays around 😄

@ko1
Copy link
Collaborator

ko1 commented Nov 24, 2022

But it is already documented...

RUBY_DEBUG_COMMANDS (commands): debug commands invoked at first stop. commands should be separated by ';;'

So I can accept to describe about it (first part of this patch)

@binarygit
Copy link
Contributor Author

;; is temporary syntax and I'm not sure we should continue to maintain it.
This is why I didn't documented it.
For the pre:, do: command, I think "\n" (newline) is better.

if the ;; syntax is temporary, and you recommend using the "\n" to separate the commands then I will change the example to use "\n". Also a little info about ;; being temporary might help other people who use it to adopt "\n", wdy think?

@st0012
Copy link
Member

st0012 commented Nov 25, 2022

But it is already documented...

IMO, it's not very clear that RUBY_DEBUG_COMMANDS and binding.b(do: "cmd") are the same thing. So it's worth mentioning this again in the binding.b API section.

For the pre:, do: command, I think "\n" (newline) is better.

I assume we can still write multiple commands in the same line? Like binding.b(do: "catch StandardError \n i \n bt 10")

@ko1
Copy link
Collaborator

ko1 commented Nov 25, 2022

if the ;; syntax is temporary, and you recommend using the "\n" to separate the commands then I will change the example to use "\n". Also a little info about ;; being temporary might help other people who use it to adopt "\n", wdy think?

+1

I assume we can still write multiple commands in the same line?

Yes, but not sure it is readable.

@st0012
Copy link
Member

st0012 commented Nov 28, 2022

I think this feature has 2 types of usages:

  1. Quickly replicate a debugging script (copy the line of script and paste it in other places)
    • For example, I sometimes put multiple debugger(do: "i ;; bt 10") as an advanced puts-debugging.
  2. Record and share debugging flow with others

For 2), I think newline separator does help:

debugger do: <<~DO
  watch @a
  break foo pre: info
DO

But it's not convenient for 1) as we now need to copy and paste multiple lines instead of just one. Typing \n for inline usage could work but it's still more difficult to type than ;;. If we want to move away from ;;, I hope it'd be something easier to type than \n.

Another thing to consider is that \n still don't allow multi-level commands as mentioned in #620, which I think should be included in this discussion as well.

@binarygit
Copy link
Contributor Author

I agree with @st0012 \n is more difficult to type than ;;. If there's a way we can keep ;;, I would vote for it!

@binarygit binarygit force-pushed the show-multiple-cmd-exe-with-binding.b branch from 606d187 to f964543 Compare November 30, 2022 15:03
@ko1 ko1 merged commit 1b8e120 into ruby:master Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants