Merge pull request #393 from ko1/show_breakpoints

show valid break point lines

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2017-12-10 20:04:42 -0300 using RuboCop version 0.51.0.
+# on 2017-12-23 19:18:18 -0300 using RuboCop version 0.51.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -19,7 +19,7 @@ Metrics/ClassLength:
 Metrics/CyclomaticComplexity:
   Max: 10
 
-# Offense count: 18
+# Offense count: 20
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 19

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Master (Unreleased)
 
+### Added
+
+* Show valid breakpoint locations when invalid location given (#393, @ko1).
+
 ## 9.1.0 - 2017-08-22
 
 ### Added

lib/byebug/commands/break.rb

@@ -2,6 +2,7 @@
 require 'byebug/helpers/eval'
 require 'byebug/helpers/file'
 require 'byebug/helpers/parse'
+require 'byebug/source_file_formatter'
 
 module Byebug
   #
@@ -89,10 +90,25 @@ def add_line_breakpoint(file, line)
       end
 
       unless Breakpoint.potential_line?(fullpath, line)
-        raise(pr('break.errors.line', file: fullpath, line: line))
+        msg = pr(
+          'break.errors.line',
+          file: fullpath,
+          line: line,
+          valid_breakpoints: valid_breakpoints_for(fullpath, line)
+        )
+
+        raise(msg)
       end
 
       Breakpoint.add(fullpath, line, @match[2])
     end
+
+    def valid_breakpoints_for(path, line)
+      potential_lines = Breakpoint.potential_lines(path)
+      annotator = ->(n) { potential_lines.include?(n) ? '[B]' : '   ' }
+      source_file_formatter = SourceFileFormatter.new(path, annotator)
+
+      source_file_formatter.lines_around(line).join.chomp
+    end
   end
 end

lib/byebug/commands/list.rb

@@ -1,4 +1,5 @@
 require 'byebug/command'
+require 'byebug/source_file_formatter'
 require 'byebug/helpers/file'
 require 'byebug/helpers/parse'
 
@@ -38,9 +39,7 @@ def execute
       msg = "No sourcefile available for #{frame.file}"
       raise(msg) unless File.exist?(frame.file)
 
-      max_lines = n_lines(frame.file)
-      b, e = range(@match[2], max_lines)
-      raise('Invalid line range') unless valid_range?(b, e, max_lines)
+      b, e = range(@match[2])
 
       display_lines(b, e)
 
@@ -56,62 +55,51 @@ def execute
     #
     # Otherwise it's automatically chosen.
     #
-    def range(input, max_line)
-      size = [Setting[:listsize], max_line].min
+    def range(input)
+      return auto_range(@match[1] || '+') unless input
 
-      return set_range(size, max_line) unless input
+      b, e = parse_range(input)
+      raise('Invalid line range') unless valid_range?(b, e)
 
-      parse_range(input, size, max_line)
+      [b, e]
     end
 
-    def valid_range?(first, last, max)
-      first <= last && (1..max).cover?(first) && (1..max).cover?(last)
+    def valid_range?(first, last)
+      first <= last && (1..max_line).cover?(first) && (1..max_line).cover?(last)
     end
 
     #
     # Set line range to be printed by list
     #
-    # @param size - number of lines to be printed
-    # @param max_line - max line number that can be printed
-    #
     # @return first line number to list
     # @return last line number to list
     #
-    def set_range(size, max_line)
-      first = amend(lower(size, @match[1] || '+'), max_line - size + 1)
+    def auto_range(direction)
+      prev_line = processor.prev_line
 
-      [first, move(first, size - 1)]
+      if direction == '=' || prev_line.nil?
+        source_file_formatter.range_around(frame.line)
+      else
+        source_file_formatter.range_from(move(prev_line, size, direction))
+      end
     end
 
-    def parse_range(input, size, max_line)
+    def parse_range(input)
       first, err = get_int(lower_bound(input), 'List', 1, max_line)
       raise(err) unless first
 
       if upper_bound(input)
         last, err = get_int(upper_bound(input), 'List', 1, max_line)
         raise(err) unless last
 
-        last = amend(last, max_line)
+        last = amend_final(last)
       else
         first -= (size / 2)
       end
 
       [first, last || move(first, size - 1)]
     end
 
-    def amend(line, max_line)
-      return 1 if line < 1
-
-      [max_line, line].min
-    end
-
-    def lower(size, direction = '+')
-      prev_line = processor.prev_line
-      return frame.line - size / 2 if direction == '=' || prev_line.nil?
-
-      move(prev_line, size, direction)
-    end
-
     def move(line, size, direction = '+')
       line.send(direction, size)
     end
@@ -125,13 +113,7 @@ def move(line, size, direction = '+')
     def display_lines(min, max)
       puts "\n[#{min}, #{max}] in #{frame.file}"
 
-      File.foreach(frame.file).with_index do |line, lineno|
-        break if lineno + 1 > max
-        next unless (min..max).cover?(lineno + 1)
-
-        mark = lineno + 1 == frame.line ? '=> ' : '   '
-        puts format("#{mark}%#{max.to_s.size}d: %s", lineno + 1, line)
-      end
+      puts source_file_formatter.lines(min, max).join
     end
 
     #
@@ -160,5 +142,16 @@ def upper_bound(range)
     def split_range(str)
       str.split(/[-,]/)
     end
+
+    extend Forwardable
+
+    def_delegators :source_file_formatter, :amend_final, :size, :max_line
+
+    def source_file_formatter
+      @source_file_formatter ||= SourceFileFormatter.new(
+        frame.file,
+        ->(n) { n == frame.line ? '=>' : '  ' }
+      )
+    end
   end
 end

lib/byebug/printers/texts/base.yml

@@ -4,7 +4,7 @@ base:
 
 break:
   errors:
-    line: "Line {line} is not a valid breakpoint in file {file}"
+    line: "Line {line} is not a valid breakpoint in file {file}.\n\nValid break points are:\n{valid_breakpoints}"
     location: "Invalid breakpoint location"
     state: "We are not in a state that has an associated file"
     class: "Unknown class {klass}"

lib/byebug/source_file_formatter.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+require 'byebug/helpers/file'
+require 'byebug/setting'
+
+module Byebug
+  #
+  # Formats specific line ranges in a source file
+  #
+  class SourceFileFormatter
+    include Helpers::FileHelper
+
+    attr_reader :file, :annotator
+
+    def initialize(file, annotator)
+      @file = file
+      @annotator = annotator
+    end
+
+    def lines(min, max)
+      File.foreach(file).with_index.map do |line, lineno|
+        next unless (min..max).cover?(lineno + 1)
+
+        annotation = annotator.call(lineno + 1)
+
+        format("%s %#{max.to_s.size}d: %s", annotation, lineno + 1, line)
+      end
+    end
+
+    def lines_around(center)
+      lines(*range_around(center))
+    end
+
+    def range_around(center)
+      range_from(center - size / 2)
+    end
+
+    def range_from(min)
+      first = amend_initial(min)
+
+      [first, first + size - 1]
+    end
+
+    def amend_initial(line)
+      amend(line, max_initial_line)
+    end
+
+    def amend_final(line)
+      amend(line, max_line)
+    end
+
+    def max_initial_line
+      max_line - size + 1
+    end
+
+    def max_line
+      @max_line ||= n_lines(file)
+    end
+
+    def size
+      [Setting[:listsize], max_line].min
+    end
+
+    def amend(line, ceiling)
+      [ceiling, [1, line].max].min
+    end
+  end
+end

test/commands/break_test.rb

@@ -287,12 +287,20 @@ def test_setting_breakpoint_to_nonexistent_line_shows_an_error
       check_error_includes "There are only 16 lines in file #{example_path}"
     end
 
-    def test_setting_breakpoint_to_invalid_line_shows_an_error
+    def test_setting_breakpoint_to_invalid_line_shows_an_error_and_alternatives
       enter 'break 14'
       debug_code(program)
 
       check_error_includes \
-        "Line 14 is not a valid breakpoint in file #{example_path}"
+        "Line 14 is not a valid breakpoint in file #{example_path}.",
+        'Valid break points are:',
+        '[B] 10:     end',
+        '[B] 11:   end',
+        '12:',
+        '[B] 13:   byebug',
+        '14:',
+        '[B] 15:   ByebugTestClass.a',
+        '[B] 16: end'
     end
   end