Breakpoints should respect the skip_path config

lib/debug/breakpoint.rb

@@ -4,6 +4,8 @@
 
 module DEBUGGER__
   class Breakpoint
+    include SkipPathHelper
+
     attr_reader :key
 
     def initialize do_enable = true
@@ -79,6 +81,14 @@ def duplicable?
       false
     end
 
+    def skip_path?(path)
+      if @path
+        !path.match?(@path)
+      else
+        super
+      end
+    end
+
     include Color
 
     def generate_label(name)
@@ -261,7 +271,6 @@ def inspect
 
   class CatchBreakpoint < Breakpoint
     attr_reader :last_exc
-    include SkipPathHelper
 
     def initialize pat, cond: nil, command: nil, path: nil
       @pat = pat.freeze
@@ -279,12 +288,7 @@ def setup
       @tp = TracePoint.new(:raise){|tp|
         exc = tp.raised_exception
         next if SystemExit === exc
-
-        if @path
-          next if !tp.path.match?(@path)
-        elsif skip_path?(tp.path)
-          next
-        end
+        next if skip_path?(tp.path)
 
         next if !safe_eval(tp.binding, @cond) if @cond
         should_suspend = false
@@ -323,7 +327,7 @@ def setup
         next if tp.path.start_with? __dir__
         next if tp.path.start_with? '<internal:'
         next if ThreadClient.current.management?
-        next if @path && !tp.path.match?(@path)
+        next if skip_path?(tp.path)
 
         if safe_eval tp.binding, @expr
           suspend
@@ -350,14 +354,14 @@ def initialize ivar, object, current, cond: nil, command: nil, path: nil
       super()
     end
 
-    def watch_eval
+    def watch_eval(tp)
       result = @object.instance_variable_get(@ivar)
       if result != @current
         begin
           @prev = @current
           @current = result
 
-          if @cond.nil? || @object.instance_eval(@cond)
+          if (@cond.nil? || @object.instance_eval(@cond)) && !skip_path?(tp.path)
             suspend
           end
         ensure
@@ -372,9 +376,8 @@ def setup
       @tp = TracePoint.new(:line, :return, :b_return){|tp|
         next if tp.path.start_with? __dir__
         next if tp.path.start_with? '<internal:'
-        next if @path && !tp.path.match?(@path)
 
-        watch_eval
+        watch_eval(tp)
       }
     end
 
@@ -416,8 +419,8 @@ def setup
         next if @cond_class && !tp.self.kind_of?(@cond_class)
 
         caller_location = caller_locations(2, 1).first.to_s
-        next if @path && !caller_location.match?(@path)
         next if caller_location.start_with?(__dir__)
+        next if skip_path?(caller_location)
 
         suspend
       }

test/debug/break_test.rb

@@ -233,6 +233,32 @@ def test_break_only_stops_when_path_matches
           end
         end
       end
+
+      def test_the_path_option_supersede_skip_path_config
+        # skips the extra_file's breakpoint
+        with_extra_tempfile do |extra_file|
+          debug_code(program(extra_file.path)) do
+            type "config set skip_path /#{extra_file.path}/"
+            type "break Foo#bar"
+            type 'c'
+            type 'up'
+            assert_line_num(5) # from the main file
+            type 'c'
+          end
+        end
+
+        # ignores skip_path and stops at designated path
+        with_extra_tempfile do |extra_file|
+          debug_code(program(extra_file.path)) do
+            type "config set skip_path /#{extra_file.path}/"
+            type "break Foo#bar path: #{extra_file.path}"
+            type 'c'
+            type 'up'
+            assert_line_num(1) # from the extra_file
+            type 'c'
+          end
+        end
+      end
     end
   end
 
@@ -642,6 +668,37 @@ def test_conditional_breakpoint_only_stops_when_path_matches
           end
         end
       end
+
+      def test_the_path_option_supersede_skip_path_config
+        # skips the extra_file's breakpoint
+        with_extra_tempfile do |extra_file|
+          debug_code(program(extra_file.path)) do
+            type "config set skip_path /#{extra_file.path}/"
+            type "break if: b == 1"
+            type 'c'
+            type 'a + b'
+            assert_line_num(3)
+            assert_line_text(/201/)
+            type 'c'
+            # this stop is because check bp stops at every line after the condition is fulfilled, which is another issue
+            # see https://github.com/ruby/debug/issues/406 for more about the issue
+            assert_line_num(4)
+            type 'c'
+          end
+        end
+
+        # ignores skip_path and stops at designated path
+        with_extra_tempfile do |extra_file|
+          debug_code(program(extra_file.path)) do
+            type "config set skip_path /#{extra_file.path}/"
+            type "break if: b == 1 path: #{extra_file.path}"
+            type 'c'
+            type 'a + b'
+            assert_line_text(/101/)
+            type 'c'
+          end
+        end
+      end
     end
   end
 end

test/debug/catch_test.rb

@@ -185,14 +185,19 @@ def test_catch_only_stops_when_path_matches
     end
 
     def test_the_path_option_supersede_skip_path_config
+      # skips the extra_file's breakpoint
       with_extra_tempfile do |extra_file|
         debug_code(program(extra_file.path)) do
-          type "config set skip_path #{extra_file.path}"
+          type "config set skip_path /#{extra_file.path}/"
+          type "catch RuntimeError"
+          type 'c'
+          assert_line_text(/foo/)
           type 'c'
         end
 
+        # ignores skip_path and stops at designated path
         debug_code(program(extra_file.path)) do
-          type "config set skip_path #{extra_file.path}"
+          type "config set skip_path /#{extra_file.path}/"
           type "catch RuntimeError path: #{extra_file.path}"
           type 'c'
           assert_line_text(/bar/)

test/debug/watch_test.rb

@@ -118,7 +118,7 @@ def program(extra_file_path)
          2|   attr_accessor :age
          3|
          4|   def initialize
-         5|     binding.b(do: "watch @age path: #{extra_file_path}")
+         5|
          6|   end
          7| end
          8|
@@ -134,6 +134,35 @@ def program(extra_file_path)
       def test_watch_only_stops_when_path_matches
         with_extra_tempfile do |extra_file|
           debug_code(program(extra_file.path)) do
+            type 'b 5'
+            type 'c'
+            type "watch @age path: #{extra_file.path}"
+            type 'c'
+            assert_line_text(/@age =  -> 25/)
+            type 'c'
+          end
+        end
+      end
+
+      def test_the_path_option_supersede_skip_path_config
+        # skips the extra_file's breakpoint
+        with_extra_tempfile do |extra_file|
+          debug_code(program(extra_file.path)) do
+            type "config set skip_path /#{extra_file.path}/"
+            type 'b 5'
+            type 'c'
+            type "watch @age"
+            type 'c'
+            assert_line_text(/@age = 25 -> 30/)
+            type 'c'
+          end
+
+          # ignores skip_path and stops at designated path
+          debug_code(program(extra_file.path)) do
+            type "config set skip_path /#{extra_file.path}/"
+            type 'b 5'
+            type 'c'
+            type "watch @age path: #{extra_file.path}"
             type 'c'
             assert_line_text(/@age =  -> 25/)
             type 'c'