Better messages for breakpoint management

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Master (Unreleased)
 
+### Added
+
+* Better UI messages for breakpoint management.
+
 ## 9.0.6 - 2016-09-29
 
 ### Fixed

lib/byebug/commands/delete.rb

@@ -44,8 +44,10 @@ def execute
 
         return errmsg(err) unless pos
 
-        unless Breakpoint.remove(pos)
-          return errmsg(pr('break.errors.no_breakpoint_delete', pos: pos))
+        if Breakpoint.remove(pos)
+          puts(pr('break.messages.breakpoint_deleted', pos: pos))
+        else
+          errmsg(pr('break.errors.no_breakpoint_delete', pos: pos))
         end
       end
     end

lib/byebug/helpers/toggle.rb

@@ -9,50 +9,51 @@ module ToggleHelper
       include ParseHelper
 
       def enable_disable_breakpoints(is_enable, args)
-        return errmsg(pr('toggle.errors.no_breakpoints')) if Breakpoint.none?
+        raise pr('toggle.errors.no_breakpoints') if Breakpoint.none?
 
-        all_breakpoints = Byebug.breakpoints.sort_by(&:id)
-        if args.nil?
-          selected_breakpoints = all_breakpoints
-        else
-          selected_ids = []
-          args.split(/ +/).each do |pos|
-            last_id = all_breakpoints.last.id
-            pos, err = get_int(pos, "#{is_enable} breakpoints", 1, last_id)
-            return errmsg(err) unless pos
-
-            selected_ids << pos
-          end
-          selected_breakpoints = all_breakpoints.select do |b|
-            selected_ids.include?(b.id)
-          end
-        end
-
-        selected_breakpoints.each do |b|
+        select_breakpoints(is_enable, args).each do |b|
           enabled = ('enable' == is_enable)
           if enabled && !syntax_valid?(b.expr)
-            return errmsg(pr('toggle.errors.expression', expr: b.expr))
+            raise pr('toggle.errors.expression', expr: b.expr)
           end
 
+          puts pr('toggle.messages.toggled', bpnum: b.id,
+                                             endis: enabled ? 'en' : 'dis')
           b.enabled = enabled
         end
       end
 
       def enable_disable_display(is_enable, args)
-        return errmsg(pr('toggle.errors.no_display')) if n_displays.zero?
+        raise pr('toggle.errors.no_display') if n_displays.zero?
 
         selected_displays = args ? args.split(/ +/) : [1..n_displays + 1]
 
         selected_displays.each do |pos|
           pos, err = get_int(pos, "#{is_enable} display", 1, n_displays)
-          return errmsg(err) unless err.nil?
+          raise err unless err.nil?
 
           Byebug.displays[pos - 1][0] = ('enable' == is_enable)
         end
       end
 
       private
 
+      def select_breakpoints(is_enable, args)
+        all_breakpoints = Byebug.breakpoints.sort_by(&:id)
+        return all_breakpoints if args.nil?
+
+        selected_ids = []
+        args.split(/ +/).each do |pos|
+          last_id = all_breakpoints.last.id
+          pos, err = get_int(pos, "#{is_enable} breakpoints", 1, last_id)
+          raise(ArgumentError, err) unless pos
+
+          selected_ids << pos
+        end
+
+        all_breakpoints.select { |b| selected_ids.include?(b.id) }
+      end
+
       def n_displays
         Byebug.displays.size
       end

lib/byebug/printers/texts/base.yml

@@ -16,6 +16,8 @@ break:
     not_changed: "Incorrect expression \"{expr}\", breakpoint not changed"
   confirmations:
     delete_all: "Delete all breakpoints?"
+  messages:
+    breakpoint_deleted: "Deleted breakpoint {pos}"
 
 catch:
   added: "Catching exception {exception}."
@@ -97,6 +99,8 @@ toggle:
     no_display: "No display expressions have been set"
     syntax: "\"{toggle}\" must be followed by \"display\", \"breakpoints\" or breakpoint ids"
     expression: "Expression \"{expr}\" syntactically incorrect; breakpoint remains disabled."
+  messages:
+    toggled: "Breakpoint {bpnum} {endis}abled"
 
 parse:
   errors:

lib/byebug/printers/texts/plain.yml

@@ -1,5 +1,5 @@
 break:
-  created: "Successfully created breakpoint with id {id}"
+  created: "Created breakpoint {id} at {file}:{line}"
 
 display:
   result: "{n}: {exp} = {result}"

test/commands/break_test.rb

@@ -84,7 +84,7 @@ def test_setting_breakpoint_to_an_undefined_class_creates_breakpoint
       enter 'break B.a'
       debug_code(program)
 
-      check_output_includes(/Successfully created breakpoint with id/)
+      check_output_includes(/Created breakpoint/)
     end
 
     def test_setting_breakpoint_to_an_undefined_class_shows_error_message
@@ -180,15 +180,15 @@ def test_setting_breakpoint_with_relative_path_adds_the_breakpoint
       enter 'break ./test/commands/break_test.rb:8'
       debug_code(program)
 
-      check_output_includes(/Successfully created breakpoint with id/)
+      check_output_includes(/Created breakpoint/)
     end
 
     def test_setting_breakpoint_with_space_in_path_adds_the_breakpoint
       with_new_file('hello world.rb', 'puts "Hello World!"') do
         enter 'break hello world.rb:1'
         debug_code(program)
 
-        check_output_includes(/Successfully created breakpoint with id/)
+        check_output_includes(/Created breakpoint/)
       end
     end
 
@@ -277,7 +277,7 @@ def test_setting_breakpoint_prints_confirmation_message
       enter 'break 7'
       debug_code(program) { @id = Breakpoint.first.id }
 
-      check_output_includes("Successfully created breakpoint with id #{@id}")
+      check_output_includes(/Created breakpoint #{@id}/)
     end
 
     def test_setting_breakpoint_to_nonexistent_line_shows_an_error

test/commands/condition_test.rb

@@ -17,13 +17,7 @@ def program
       EOC
     end
 
-    def test_setting_condition_w_short_syntax_assigns_expression_to_breakpoint
-      enter 'break 5', -> { "cond #{Breakpoint.first.id} b == 5" }
-
-      debug_code(program) { assert_equal 'b == 5', Breakpoint.first.expr }
-    end
-
-    def test_setting_condition_w_full_syntax_assigns_expression_to_breakpoint
+    def test_setting_condition_assigns_expression_to_breakpoint
       enter 'break 5', -> { "condition #{Breakpoint.first.id} b == 5" }
 
       debug_code(program) { assert_equal 'b == 5', Breakpoint.first.expr }

test/commands/delete_test.rb

@@ -31,6 +31,13 @@ def test_deleting_a_breakpoint_removes_it_from_breakpoints_list
       debug_code(program) { assert_empty Byebug.breakpoints }
     end
 
+    def test_deleting_a_breakpoint_shows_a_success_message
+      enter 'break 9', -> { "delete #{Breakpoint.first.id}" }
+      debug_code(program)
+
+      check_output_includes(/Deleted breakpoint/)
+    end
+
     def test_does_not_stop_at_the_deleted_breakpoint
       enter 'b 9', 'b 10', -> { "delete #{Breakpoint.first.id}" }, 'cont'
 

test/commands/disable_test.rb

@@ -33,45 +33,58 @@ def program
       EOC
     end
 
-    def test_disable_breakpoints_with_short_syntax_sets_enabled_to_false
-      enter 'break 21', 'break 22', -> { "disable b #{Breakpoint.first.id}" }
+    def test_disable_all_breakpoints_sets_all_enabled_flags_to_false
+      enter 'break 21', 'break 22', 'disable breakpoints'
 
-      debug_code(program) { assert_equal false, Breakpoint.first.enabled? }
+      debug_code(program) do
+        assert_equal false, Breakpoint.first.enabled?
+        assert_equal false, Breakpoint.last.enabled?
+      end
     end
 
-    def test_disable_breakpoints_with_short_syntax_properly_ignores_them
-      enter 'b 21', 'b 22', -> { "disable b #{Breakpoint.first.id}" }, 'cont'
+    def test_disable_all_breakpoints_shows_success_messages_for_all_breakpoints
+      enter 'break 21', 'break 22', 'disable breakpoints'
+      debug_code(program)
 
-      debug_code(program) { assert_equal 22, frame.line }
+      check_output_includes(/Breakpoint #{Breakpoint.first.id} disabled/,
+                            /Breakpoint #{Breakpoint.last.id} disabled/)
+    end
+
+    def test_disable_all_breakpoints_ignores_all_breakpoints
+      enter 'break 21', 'break 22', 'disable breakpoints', 'cont'
+      debug_code(program)
+
+      check_output_doesnt_include 'Stopped by breakpoint'
     end
 
-    def test_disable_breakpoints_with_full_syntax_sets_enabled_to_false
+    def test_disable_specific_breakpoints_sets_enabled_to_false
       enter 'b 21', 'b 22', -> { "disable breakpoints #{Breakpoint.first.id}" }
 
       debug_code(program) { assert_equal false, Breakpoint.first.enabled? }
     end
 
-    def test_disable_breakpoints_with_full_syntax_properly_ignores_them
+    def test_disable_specific_breakpoints_shows_success_message
       enter 'break 21', 'break 22',
-            -> { "disable breakpoints #{Breakpoint.first.id}" }, 'cont'
+            -> { "disable breakpoints #{Breakpoint.first.id}" }
+      debug_code(program)
 
-      debug_code(program) { assert_equal 22, frame.line }
+      check_output_includes(/Breakpoint #{Breakpoint.first.id} disabled/)
     end
 
-    def test_disable_all_breakpoints_sets_all_enabled_flags_to_false
-      enter 'break 21', 'break 22', 'disable breakpoints'
+    def test_disable_specific_breakpoints_properly_ignores_them
+      enter 'break 21', 'break 22',
+            -> { "disable breakpoints #{Breakpoint.first.id}" }, 'cont'
 
-      debug_code(program) do
-        assert_equal false, Breakpoint.first.enabled?
-        assert_equal false, Breakpoint.last.enabled?
-      end
+      debug_code(program) { assert_equal 22, frame.line }
     end
 
-    def test_disable_all_breakpoints_ignores_all_breakpoints
-      enter 'break 21', 'break 22', 'disable breakpoints', 'cont'
+    def test_disable_with_an_incorrect_breakpoint_number_shows_error
+      enter 'break 21', 'break 22',
+            -> { "disable breakpoints #{Breakpoint.last.id + 1}" }
       debug_code(program)
 
-      check_output_doesnt_include 'Stopped by breakpoint'
+      assert_equal 1, interface.error.size
+      check_error_includes(/"disable breakpoints" argument/)
     end
 
     def test_disable_without_an_argument_shows_help

test/commands/enable_test.rb

@@ -33,20 +33,6 @@ def program
       EOC
     end
 
-    def test_enable_breakpoints_with_short_syntax_sets_enabled_to_true
-      enter 'b 21', 'b 22', 'disable breakpoints',
-            -> { "enable b #{Breakpoint.first.id}" }
-
-      debug_code(program) { assert_equal true, Breakpoint.first.enabled? }
-    end
-
-    def test_enable_breakpoints_with_short_syntax_stops_at_enabled_breakpoint
-      enter 'break 21', 'break 22', 'disable breakpoints',
-            -> { "enable b #{Breakpoint.first.id}" }, 'cont'
-
-      debug_code(program) { assert_equal 21, frame.line }
-    end
-
     def test_enable_all_breakpoints_sets_all_enabled_flags_to_true
       enter 'break 21', 'break 22', 'disable breakpoints', 'enable breakpoints'
 
@@ -56,6 +42,14 @@ def test_enable_all_breakpoints_sets_all_enabled_flags_to_true
       end
     end
 
+    def test_enable_all_breakpoints_shows_success_messages_for_all_breakpoints
+      enter 'break 21', 'break 22', 'disable breakpoints', 'enable breakpoints'
+      debug_code(program)
+
+      check_output_includes(/Breakpoint #{Breakpoint.first.id} enabled/,
+                            /Breakpoint #{Breakpoint.last.id} enabled/)
+    end
+
     def test_enable_all_breakpoints_stops_at_first_breakpoint
       enter 'b 21', 'b 22', 'disable breakpoints', 'enable breakpoints', 'cont'
 
@@ -69,20 +63,37 @@ def test_enable_all_breakpoints_stops_at_last_breakpoint
       debug_code(program) { assert_equal 22, frame.line }
     end
 
-    def test_enable_breakpoints_with_full_syntax_sets_enabled_to_false
+    def test_enable_specific_breakpoints_sets_enabled_to_true
+      enter 'break 21', 'break 22', 'disable breakpoints',
+            -> { "enable breakpoints #{Breakpoint.last.id}" }
+
+      debug_code(program) { assert_equal true, Breakpoint.last.enabled? }
+    end
+
+    def test_enable_specific_breakpoints_shows_success_message
       enter 'break 21', 'break 22', 'disable breakpoints',
             -> { "enable breakpoints #{Breakpoint.last.id}" }
+      debug_code(program)
 
-      debug_code(program) { assert_equal false, Breakpoint.first.enabled? }
+      check_output_includes(/Breakpoint #{Breakpoint.last.id} enabled/)
     end
 
-    def test_enable_breakpoints_with_full_syntax_stops_at_enabled_breakpoint
+    def test_enable_specific_breakpoints_stops_at_enabled_breakpoint
       enter 'break 21', 'break 22', 'disable breakpoints',
             -> { "enable breakpoints #{Breakpoint.last.id}" }, 'cont'
 
       debug_code(program) { assert_equal 22, frame.line }
     end
 
+    def test_enable_with_an_incorrect_breakpoint_number_shows_error
+      enter 'break 21', 'break 22', 'disable breakpoints',
+            -> { "enable breakpoints #{Breakpoint.last.id + 1}" }
+      debug_code(program)
+
+      assert_equal 1, interface.error.size
+      check_error_includes(/"enable breakpoints" argument/)
+    end
+
     def test_enable_by_itself_shows_help
       enter 'enable'
       debug_code(program)