Refactor (3/5): consolidate duplicated helpers, guard OwnSource::FILES

CLAUDE.md

@@ -186,7 +186,9 @@ table is affected. `mrblib/mrdebug/line_breakpoint.rb`'s suffix match and
   TCP instead. It also colors its output (`Colorizer.colorize_line`, the
   `tools/mrdebug/{ruby_highlighter,colorizer}.rb` files its `mrbgem.rake`
   adds to its own `rbfiles`) by default (no TTY probe — a serial terminal can't be probed;
-  `MRDEBUG_COLOR=never` or `NO_COLOR` turns it off), and not the prompt, since
+  `MRDEBUG_COLOR=never`, `NO_COLOR` or `TERM=dumb` turns it off —
+  `Colorizer.enabled?(nil)`, a `nil` io meaning "nothing to probe"), and
+  not the prompt, since
   `Editor::Line` sizes it by character count.
 - **`src/hook.c`** — the VM hook. `struct mrdebug_hook hook` (file-static)
   holds everything: the installed session, whether the hook is armed,
@@ -333,6 +335,11 @@ table is affected. `mrblib/mrdebug/line_breakpoint.rb`'s suffix match and
     line breakpoints (one shared number sequence) and calls
     **`#sync_method_names`**, which pushes the active method breakpoints'
     name symbols to `Hook.watch_method_names` (also on remove/clear).
+- **`mrblib/mrdebug/breakpoint.rb`** — `MRDebug::Breakpoint`, the mixin
+  all three breakpoint classes include: `active?`/`deactivate!`,
+  `numbered_line`, and `number_in(siblings)` (its index in `Session`'s
+  list + 1, which `#stop_banner` prints). A `WatchVarBreakpoint` is never
+  deactivated (watches can't be deleted).
 - **`mrblib/mrdebug/line_breakpoint.rb`** — file/line/active, suffix match
   via hand-rolled `String#[]` slicing (see "Avoid `mruby-string-ext`
   methods" above), stable numbering shared with `Session`'s breakpoint
@@ -416,7 +423,9 @@ table is affected. `mrblib/mrdebug/line_breakpoint.rb`'s suffix match and
 - **`tools/mrdebug/transport/socket.rb`** (host builds only) —
   `MRDebug::Transport::Socket`/`TCP`/`Unix`: `mruby-socket`, wrapping the
   accepted (`.listen`, device side) or connected (`.connect`, CLI side)
-  socket. `#gets` uses `#sysread`, not mruby-io's buffered `IO#gets` —
+  socket. `#gets` uses `#sysread` (`Transport.read_some`, also what the
+  CLI's relay, `DeviceLink` and `DapServer` read with), not mruby-io's
+  buffered `IO#gets` —
   the latter makes `#write` raise `Errno::ESPIPE` (silently swallowed by
   the VM hook, freezing the `(mrdbg)` loop) once a read has more than one
   line buffered ahead, since `IO#write` on a dual-purpose fd tries to
@@ -438,9 +447,11 @@ table is affected. `mrblib/mrdebug/line_breakpoint.rb`'s suffix match and
   R2P2 `MRDEBUG_PORT` comes from `/etc/config.yml`'s `env: mrdebug_port:`
   or the shell's `export`). `attach_local` is the one seam the
   `mrdebug-console` gem overrides. A listener bind failure propagates out
-  of `binding.debugger` rather than being swallowed. `env_value` tolerates
-  a build without `mruby-env` (`defined?(ENV)`) and treats a blank value
-  as unset.
+  of `binding.debugger` rather than being swallowed. `MRDebug.env_value`
+  (core, `mrblib/mrdebug.rb`, also what `Colorizer` reads) tolerates a
+  build without `mruby-env` (`defined?(ENV)`) and treats a blank value as
+  unset. `listen_tcp`/`listen_unix`/`attach_stdio` (and the console gem's
+  `attach_local`) all go through `MRDebug.start_session(ui)`.
 - **Re-attach** (`MRDebug.detach`, `mrblib/mrdebug.rb`): when a socket
   client goes away (EOF, or a failed write — `LocalConsole#lost`, gated on
   `Transport#detach_on_close?`), the stop resumes, the session is dropped
@@ -535,9 +546,9 @@ table is affected. `mrblib/mrdebug/line_breakpoint.rb`'s suffix match and
   `(mrdbg) ` prompt once whole. `CLI.relay` feeds device bytes through it;
   `Transport::Colored` is the same as a `Transport` decorator for
   `MRDebug.attach_stdio` (`stdio_transport`). `Colorizer.enabled?`: `--color`
-  / `MRDEBUG_COLOR`, then `NO_COLOR`/`TERM=dumb`, then `io.tty?`. `cat` and
-  `p` output aren't colored (no way to tell them from other text). Add new
-  files to `OwnSource::FILES`.
+  / `MRDEBUG_COLOR`, then `NO_COLOR`/`TERM=dumb`, then `io.tty?` (a `nil`
+  io counts as yes). `cat` and `p` output aren't colored (no way to tell
+  them from other text).
 - **`tools/mrdebug/ui/local_console.rb`** (host builds only) —
   `MRDebug::UI::LocalConsole`: the `(mrdbg)` prompt, one `STDIN.gets` (now via
   a `Transport`, defaulting to `Stdio`) per command. Reading exactly one
@@ -557,7 +568,8 @@ table is affected. `mrblib/mrdebug/line_breakpoint.rb`'s suffix match and
   Hardcoded rather than discovered at runtime (`Dir.glob` would need a
   filesystem, which a PicoRuby target may not have) — update
   `OwnSource::FILES` when adding, removing, or renaming a file under
-  `mrblib/mrdebug/` or `tools/mrdebug/`. Missing an
+  `mrblib/`, `tools/mrdebug/` or `console/mrblib/` (`spec/own_source_spec.rb`
+  compares the list against the tree and fails until it matches). Missing an
   entry shows up as `step N`/`next N`'s counter being consumed by mrdebug's
   own code, or `watch` reporting a shifted line — symptoms that look like
   VM bugs but aren't.

console/mrblib/mrdebug/console.rb

@@ -11,11 +11,10 @@ module UI
     class Console < Base
       def initialize
         # On by default (a serial terminal can't be probed for ANSI support,
-        # so there is no TTY check); MRDEBUG_COLOR=never or NO_COLOR turns it
-        # off. The prompt itself stays plain: Editor::Line sizes it by
+        # so there is no TTY check); MRDEBUG_COLOR=never, NO_COLOR or
+        # TERM=dumb turns it off. The prompt itself stays plain: Editor::Line sizes it by
         # character count.
-        mode = Colorizer.env('MRDEBUG_COLOR')
-        @color = mode == 'always' || (mode != 'never' && Colorizer.env('NO_COLOR').nil?)
+        @color = Colorizer.enabled?(nil)
         @editor = Class.new(Editor::Line) do
           def initialize
             @height, @width = 24, 80
@@ -63,9 +62,6 @@ def say(line)
   # Replaces tools/mrdebug/device.rb's attach_local (this gem loads later,
   # as a dependent): what MRDEBUG_PORT=console selects on a device.
   def self.attach_local
-    session = Session.new
-    session.ui = UI::Console.new
-    self.session = session
-    session
+    start_session(UI::Console.new)
   end
 end

mrblib/mrdebug.rb

@@ -17,6 +17,13 @@ def self.error_text(e)
     "#{e.class}: #{e.message}"
   end
 
+  # ENV[name], or nil when it's unset, blank, or there's no ENV at all
+  # (a build without mruby-env/picoruby-env).
+  def self.env_value(name)
+    value = defined?(ENV) ? ENV[name] : nil
+    value && !value.empty? ? value : nil
+  end
+
   # Overridden on host builds (tools/mrdebug/device.rb); a no-op otherwise.
   def self.autostart
   end

mrblib/mrdebug/breakpoint.rb

@@ -0,0 +1,24 @@
+module MRDebug
+  # Shared by LineBreakpoint, MethodBreakpoint and WatchVarBreakpoint.
+  # A breakpoint's number is its index in Session's list + 1, so `delete`
+  # deactivates in place rather than removing (later numbers stay put).
+  # Watches can't be deleted, so a WatchVarBreakpoint is always active.
+  module Breakpoint
+    def active?
+      !@deactivated
+    end
+
+    def deactivate!
+      @deactivated = true
+    end
+
+    # This breakpoint's number within `siblings` (the Session list it's in).
+    def number_in(siblings)
+      siblings.index(self) + 1
+    end
+
+    def numbered_line(index)
+      "  ##{index} #{self}"
+    end
+  end
+end

mrblib/mrdebug/line_breakpoint.rb

@@ -1,36 +1,25 @@
 module MRDebug
   class LineBreakpoint
+    include Breakpoint
+
     attr_reader :file, :line, :condition
 
     def initialize(file, line, condition = nil)
       @file = file
       @line = line
       @condition = condition
-      @active = true
-    end
-
-    def active?
-      @active
-    end
-
-    def deactivate!
-      @active = false
     end
 
     def match?(file, line)
       active? && @line == line && file[-@file.size, @file.size] == @file
     end
 
     def stop_banner(siblings, location)
-      "#{Protocol::BREAKPOINT}#{siblings.index(self) + 1}: #{location}"
+      "#{Protocol::BREAKPOINT}#{number_in(siblings)}: #{location}"
     end
 
     def to_s
       condition ? "#{file}:#{line} if #{condition}" : "#{file}:#{line}"
     end
-
-    def numbered_line(index)
-      "  ##{index} #{self}"
-    end
   end
 end

mrblib/mrdebug/method_breakpoint.rb

@@ -3,6 +3,8 @@ module MRDebug
   # hook (src/hook.c decodes OP_SEND). No resolution to file/line: the class
   # need not exist yet. Lives in Session's @breakpoints next to LineBreakpoint.
   class MethodBreakpoint
+    include Breakpoint
+
     attr_reader :class_name, :method_name, :condition
 
     # class_name: "Foo" / "Foo::Bar" / nil (any class). singleton: true for
@@ -12,18 +14,9 @@ def initialize(class_name, method_name, singleton = false, condition = nil)
       @method_name = method_name
       @singleton = singleton
       @condition = condition
-      @active = true
       @cfunc = false
     end
 
-    def active?
-      @active
-    end
-
-    def deactivate!
-      @active = false
-    end
-
     def method_sym
       @method_name.to_sym
     end
@@ -38,7 +31,7 @@ def match?(_file, _line)
     # bar runs on any Foo (subclasses and module includers included);
     # `Foo.bar` matches when bar runs on Foo itself.
     def matches_call?(recv)
-      return false unless @active
+      return false unless active?
       return true if @class_name.nil?
       klass = MethodBreakpoint.resolve(@class_name)
       return false unless klass
@@ -50,7 +43,7 @@ def cfunc!
     end
 
     def stop_banner(siblings, location)
-      line = "#{Protocol::BREAKPOINT}#{siblings.index(self) + 1}: #{self}"
+      line = "#{Protocol::BREAKPOINT}#{number_in(siblings)}: #{self}"
       @cfunc ? "#{line} (about to call a C method)" : line
     end
 
@@ -59,10 +52,6 @@ def to_s
       @condition ? "#{s} if #{@condition}" : s
     end
 
-    def numbered_line(index)
-      "  ##{index} #{self}"
-    end
-
     # "Foo::Bar" -> the constant, or nil if it isn't defined yet.
     def self.resolve(name)
       name.split('::').reduce(Object) { |mod, part| mod.const_get(part) }

mrblib/mrdebug/own_source.rb

@@ -1,11 +1,13 @@
 module MRDebug
-  # This gem's own Ruby source files, so Session can exclude them from
-  # tracing. Hardcoded, not discovered via Dir.glob (a PicoRuby target may
-  # have no filesystem) -- update FILES when the file list changes.
+  # This gem's own Ruby source files (and mrdebug-console's), so Session
+  # can exclude them from tracing. Hardcoded, not discovered via Dir.glob (a
+  # PicoRuby target may have no filesystem) -- update FILES when the file
+  # list changes; spec/own_source_spec.rb fails until it matches.
   module OwnSource
     FILES = [
       'mrblib/mrdebug.rb',
       'mrblib/binding.rb',
+      'mrblib/mrdebug/breakpoint.rb',
       'mrblib/mrdebug/command.rb',
       'mrblib/mrdebug/display_expression.rb',
       'mrblib/mrdebug/line_breakpoint.rb',
@@ -17,11 +19,22 @@ module OwnSource
       'mrblib/mrdebug/transport/loopback.rb',
       'mrblib/mrdebug/ui.rb',
       'mrblib/mrdebug/watch_var_breakpoint.rb',
+      'tools/mrdebug/cli/cli.rb',
+      'tools/mrdebug/cli/dap_bridge.rb',
+      'tools/mrdebug/cli/dap_server.rb',
+      'tools/mrdebug/cli/device_link.rb',
+      'tools/mrdebug/cli/json.rb',
+      'tools/mrdebug/cli/main.rb',
+      'tools/mrdebug/cli/options.rb',
       'tools/mrdebug/colorizer.rb',
+      'tools/mrdebug/device.rb',
+      'tools/mrdebug/remote_session.rb',
       'tools/mrdebug/ruby_highlighter.rb',
       'tools/mrdebug/transport/colored.rb',
+      'tools/mrdebug/transport/socket.rb',
       'tools/mrdebug/transport/stdio.rb',
       'tools/mrdebug/ui/local_console.rb',
+      'console/mrblib/mrdebug/console.rb',
     ]
 
     # Suffix match, same idiom as LineBreakpoint#match? -- the VM reports

mrblib/mrdebug/watch_var_breakpoint.rb

@@ -1,5 +1,7 @@
 module MRDebug
   class WatchVarBreakpoint
+    include Breakpoint
+
     UNSET = Object.new
 
     attr_reader :expr
@@ -19,17 +21,13 @@ def changed?(bnd)
     end
 
     def stop_banner(siblings, location)
-      "#{Protocol::WATCHPOINT}#{siblings.index(self) + 1}: #{location}"
+      "#{Protocol::WATCHPOINT}#{number_in(siblings)}: #{location}"
     end
 
     def to_s
       "watch: #{expr}"
     end
 
-    def numbered_line(index)
-      "  ##{index} #{self}"
-    end
-
     private
 
     def safe_eval(bnd)

spec/own_source_spec.rb

@@ -0,0 +1,18 @@
+
+# OwnSource::FILES is hardcoded (a device may have no filesystem to glob),
+# so nothing else notices when a file is added, removed or renamed.
+RSpec.describe 'MRDebug::OwnSource::FILES' do
+  it "lists exactly this repo's Ruby sources" do
+    root = File.expand_path('..', __dir__)
+    expected = Dir.chdir(root) do
+      Dir.glob(['mrblib/**/*.rb', 'tools/mrdebug/**/*.rb', 'console/mrblib/**/*.rb']).sort
+    end
+    out = Dir.mktmpdir do |dir|
+      File.write(File.join(dir, 'files.rb'), "puts MRDebug::OwnSource::FILES\n")
+      stdout, status = Open3.capture2(smoke_bin, 'files.rb', chdir: dir)
+      expect(status).to be_success
+      stdout
+    end
+    expect(out.split("\n").sort).to eq(expected)
+  end
+end