Skip to content

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jul 4, 2022

This is a leftover issue caused by #642

When a remote debugger client is started, it doesn't have configurations filled like the debuggee does, which can cause some issues. Like:

disconnected (undefined method `empty?' for nil:NilClass

      if !history_file.empty?
                      ^^^^^^^)

This is because CONFIG.update replaces the entire config hash and removes all the default values CONFIG already has. So if we start a client with rdbg -A, this will happen:

  1. config = DEBUGGER__::Config::parse_argv(ARGV) returns a hash like { mode: :attach }.
    • The mode isn't a real configuration option. It's only used to pass the mode information from the -A flag.
  2. The CONFIG will be initialized with all the default configuration values.
    • {:mode=>:start, :log_level=>:WARN, :show_src_lines=>10, :show_frames=>2, :use_short_path=>false, .....}
  3. CONFIG.update will replace the hash in 2 with the { mode: :attach }.
  4. The client loses all the default configurations.

mode is not an actual configuration option, it's only used to carry
the result of parse_argv to exe/rdbg's case statement.

Having it in the config hash will cause confusion and make
`CONFIG.set_config(config)` fail.
@st0012 st0012 force-pushed the fix-remote-client-configuration branch from c136bb2 to 82f7192 Compare July 4, 2022 12:08
`update` replaces the hash directly and can drop the populated default values,
while `set_config` keeps the value `CONFIG` already has and only sets the differences.
@st0012 st0012 force-pushed the fix-remote-client-configuration branch from 82f7192 to 021a1ea Compare July 4, 2022 12:11
@st0012
Copy link
Member Author

st0012 commented Jul 4, 2022

cc @peterzhu2118

Copy link
Member

@peterzhu2118 peterzhu2118 left a comment

Choose a reason for hiding this comment

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

Looks good!

@peterzhu2118 peterzhu2118 merged commit 64acbe5 into ruby:master Jul 4, 2022
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

Successfully merging this pull request may close these issues.

2 participants