Merge pull request #10 from mruwek/multi-output-consistency

More output and behavior consistency between 'del', 'do', 'pri and 'postpone'

test/DeleteCommandTest.py

@@ -133,11 +133,22 @@ class DeleteCommandTest(CommandTest.CommandTest):
         self.assertEquals(self.todolist.count(), 0)
 
     def test_multi_del3(self):
-        """ Test deletion of multiple items. """
+        """  Fail if any of supplied todo numbers is invalid. """
         command = DeleteCommand(["99", "2"], self.todolist, self.out, self.error, _yes_prompt)
         command.execute()
 
-        self.assertEquals(self.todolist.count(), 1)
+        self.assertFalse(self.todolist.is_dirty())
+        self.assertEquals(self.output, "")
+        self.assertEquals(self.errors, "Invalid todo number given: 99.\n")
+
+    def test_multi_del4(self):
+        """  Check output when all supplied todo numbers are invalid. """
+        command = DeleteCommand(["99", "A"], self.todolist, self.out, self.error, _yes_prompt)
+        command.execute()
+
+        self.assertFalse(self.todolist.is_dirty())
+        self.assertEquals(self.output, "")
+        self.assertEquals(self.errors, "Invalid todo number given: 99.\nInvalid todo number given: A.\n")
 
     def test_empty(self):
         command = DeleteCommand([], self.todolist, self.out, self.error)

test/DoCommandTest.py

@@ -312,20 +312,17 @@ class DoCommandTest(CommandTest.CommandTest):
         command = DoCommand(["99", "3"], self.todolist, self.out, self.error, _no_prompt)
         command.execute()
 
-        self.assertTrue(self.todolist.todo(3).is_completed())
-        self.assertEquals(self.output, "Completed: x {} Baz p:1\n".format(self.today))
-        self.assertEquals(self.errors, "Invalid todo number given.\n")
+        self.assertFalse(self.todolist.todo(3).is_completed())
+        self.assertEquals(self.errors, "Invalid todo number given: 99.\n")
 
     def test_multi_do5(self):
         """
-        When a todo item was generated by a recurring todo item, make sure
-        it cannot be completed in the same invocation.
+        Check output when all supplied todo numbers are invalid.
         """
-        command = DoCommand(["4", "10"], self.todolist, self.out, self.error, _no_prompt)
+        command = DoCommand(["99", "10"], self.todolist, self.out, self.error, _no_prompt)
         command.execute()
 
-        self.assertTrue(self.todolist.todo(4).is_completed())
-        self.assertFalse(self.todolist.todo(10).is_completed())
+        self.assertEquals(self.errors, "Invalid todo number given: 99.\nInvalid todo number given: 10.\n")
 
     def test_invalid_recurrence(self):
         """ Show error message when an item has an invalid recurrence pattern. """

test/PostponeCommandTest.py

@@ -211,6 +211,14 @@ class PostponeCommandTest(CommandTest.CommandTest):
         self.assertEquals(self.output, "")
         self.assertEquals(self.errors, "Invalid todo number given: 99.\nInvalid todo number given: 123.\n")
 
+    def test_postpone19(self):
+        command = PostponeCommand(["Zoo", "99", "123", "1w"], self.todolist, self.out, self.error)
+        command.execute()
+
+        self.assertFalse(self.todolist.is_dirty())
+        self.assertEquals(self.output, "")
+        self.assertEquals(self.errors, "Invalid todo number given: Zoo.\nInvalid todo number given: 99.\nInvalid todo number given: 123.\n")
+
     def test_help(self):
         command = PostponeCommand(["help"], self.todolist, self.out, self.error)
         command.execute()

test/PriorityCommandTest.py

@@ -35,7 +35,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         command.execute()
 
         self.assertTrue(self.todolist.is_dirty())
-        self.assertEquals(self.output, "Priority changed from A to B\n(B) Foo\n")
+        self.assertEquals(self.output, "Priority changed from A to B\n|  1| (B) Foo\n")
         self.assertEquals(self.errors, "")
 
     def test_set_prio2(self):
@@ -43,7 +43,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         command.execute()
 
         self.assertTrue(self.todolist.is_dirty())
-        self.assertEquals(self.output, "Priority set to Z.\n(Z) Bar\n")
+        self.assertEquals(self.output, "Priority set to Z.\n|  2| (Z) Bar\n")
         self.assertEquals(self.errors, "")
 
     def test_set_prio3(self):
@@ -51,7 +51,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         command.execute()
 
         self.assertTrue(self.todolist.is_dirty())
-        self.assertEquals(self.output, "Priority changed from A to B\n(B) Foo\n")
+        self.assertEquals(self.output, "Priority changed from A to B\n|  1| (B) Foo\n")
         self.assertEquals(self.errors, "")
 
     def test_set_prio4(self):
@@ -59,7 +59,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         command.execute()
 
         self.assertFalse(self.todolist.is_dirty())
-        self.assertEquals(self.output, "(A) Foo\n")
+        self.assertEquals(self.output, "|  1| (A) Foo\n")
         self.assertEquals(self.errors, "")
 
     def test_set_prio5(self):
@@ -67,7 +67,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         command.execute()
 
         self.assertTrue(self.todolist.is_dirty())
-        self.assertEquals(self.output, "Priority changed from A to C\n(C) Foo\nPriority set to C.\n(C) Bar\n")
+        self.assertEquals(self.output, "Priority changed from A to C\n|  1| (C) Foo\nPriority set to C.\n|  2| (C) Bar\n")
         self.assertEquals(self.errors, "")
 
     def test_invalid1(self):
@@ -79,6 +79,22 @@ class PriorityCommandTest(CommandTest.CommandTest):
         self.assertEquals(self.errors, "Invalid todo number given.\n")
 
     def test_invalid2(self):
+        command = PriorityCommand(["1", "99", "A"], self.todolist, self.out, self.error)
+        command.execute()
+
+        self.assertFalse(self.todolist.is_dirty())
+        self.assertFalse(self.output)
+        self.assertEquals(self.errors, "Invalid todo number given: 99.\n")
+
+    def test_invalid3(self):
+        command = PriorityCommand(["98", "99", "A"], self.todolist, self.out, self.error)
+        command.execute()
+
+        self.assertFalse(self.todolist.is_dirty())
+        self.assertFalse(self.output)
+        self.assertEquals(self.errors, "Invalid todo number given: 98.\nInvalid todo number given: 99.\n")
+
+    def test_invalid4(self):
         command = PriorityCommand(["1", "ZZ"], self.todolist, self.out, self.error)
         command.execute()
 
@@ -86,7 +102,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         self.assertFalse(self.output)
         self.assertEquals(self.errors, "Invalid priority given.\n")
 
-    def test_invalid3(self):
+    def test_invalid5(self):
         command = PriorityCommand(["A"], self.todolist, self.out, self.error)
         command.execute()
 
@@ -94,7 +110,7 @@ class PriorityCommandTest(CommandTest.CommandTest):
         self.assertFalse(self.output)
         self.assertEquals(self.errors, command.usage() + "\n")
 
-    def test_invalid4(self):
+    def test_invalid6(self):
         command = PriorityCommand(["1"], self.todolist, self.out, self.error)
         command.execute()
 

topydo/lib/DCommand.py

@@ -40,11 +40,12 @@ class DCommand(Command):
         self.length = len(self.todolist.todos()) # to determine newly activated todos
 
         self.todos = []
+        self.invalid_numbers = []
         for number in self.args:
             try:
                 self.todos.append(self.todolist.todo(number))
             except InvalidTodoException:
-                self.todos.append(None)
+                self.invalid_numbers.append(number)
 
     def get_flags(self):
         """ Default implementation of getting specific flags. """
@@ -141,13 +142,16 @@ class DCommand(Command):
 
         if len(self.args) == 0:
             self.error(self.usage())
+        elif len(self.invalid_numbers) > 1 or len(self.invalid_numbers) > 0 and len(self.todos) > 0:
+            for number in self.invalid_numbers:
+                self.error("Invalid todo number given: {}.".format(number))
+        elif len(self.invalid_numbers) == 1 and len(self.todos) == 0:
+            self.error("Invalid todo number given.")
         else:
             old_active = self._active_todos()
 
             for todo in self.todos:
-                if not todo:
-                    self.error("Invalid todo number given.")
-                elif todo and self.condition(todo):
+                if todo and self.condition(todo):
                     self._process_subtasks(todo)
                     self.execute_specific(todo)
                 else:

topydo/lib/PostponeCommand.py

@@ -60,22 +60,21 @@ class PostponeCommand(Command):
 
         todos = []
         invalid_numbers = []
+
         for number in self.args[:-1]:
             try:
                 todos.append(self.todolist.todo(number))
             except InvalidTodoException:
                 invalid_numbers.append(number)
 
-        if len(invalid_numbers) > 0 and len(todos) > 0:
+        if len(invalid_numbers) > 1 or len(invalid_numbers) > 0 and len(todos) > 0:
             for number in invalid_numbers:
                 self.error("Invalid todo number given: {}.".format(number))
         elif len(invalid_numbers) == 1 and len(todos) == 0:
             self.error("Invalid todo number given.")
         else:
-
             try:
                 pattern = self.args[-1]
-
                 self.printer.add_filter(PrettyPrinterNumbers(self.todolist))
 
                 if len(todos) > 0:

topydo/lib/PriorityCommand.py

@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from topydo.lib.Command import Command, InvalidCommandArgument
+from topydo.lib.PrettyPrinterFilter import PrettyPrinterNumbers
 from topydo.lib.TodoListBase import InvalidTodoException
 from topydo.lib.Utils import is_valid_priority
 
@@ -30,37 +31,44 @@ class PriorityCommand(Command):
         if not super(PriorityCommand, self).execute():
             return False
 
-        numbers = None
         priority = None
-        try:
-            numbers = self.args[:-1]
-            priority = self.args[-1]
+        todos = []
+        invalid_numbers = []
 
-            if len(numbers) > 0:
-                todos = [self.todolist.todo(number) for number in numbers]
+        for number in self.args[:-1]:
+            try:
+                todos.append(self.todolist.todo(number))
+            except InvalidTodoException:
+                invalid_numbers.append(number)
 
-                if is_valid_priority(priority):
-                    for todo in todos:
-                        old_priority = todo.priority()
-                        self.todolist.set_priority(todo, priority)
+        if len(invalid_numbers) > 1 or len(invalid_numbers) > 0 and len(todos) > 0:
+            for number in invalid_numbers:
+                self.error("Invalid todo number given: {}.".format(number))
+        elif len(invalid_numbers) == 1 and len(todos) == 0:
+            self.error("Invalid todo number given.")
+        else:
+            try:
+                priority = self.args[-1]
+                self.printer.add_filter(PrettyPrinterNumbers(self.todolist))
 
-                        if old_priority and priority and old_priority != priority:
-                            self.out("Priority changed from {} to {}".format(
-                                old_priority, priority))
-                        elif not old_priority:
-                            self.out("Priority set to {}.".format(priority))
+                if len(todos) > 0:
+                    if is_valid_priority(priority):
+                        for todo in todos:
+                            old_priority = todo.priority()
+                            self.todolist.set_priority(todo, priority)
 
-                        self.out(self.printer.print_todo(todo))
+                            if old_priority and priority and old_priority != priority:
+                                self.out("Priority changed from {} to {}".format(
+                                    old_priority, priority))
+                            elif not old_priority:
+                                self.out("Priority set to {}.".format(priority))
+
+                            self.out(self.printer.print_todo(todo))
+                    else:
+                        self.error("Invalid priority given.")
                 else:
-                    self.error("Invalid priority given.")
-            else:
-                self.error(self.usage())
-        except (IndexError, InvalidCommandArgument):
-            self.error(self.usage())
-        except (InvalidTodoException):
-            if len(numbers) > 0 and priority:
-                self.error( "Invalid todo number given.")
-            else:
+                    self.error(self.usage())
+            except (IndexError, InvalidCommandArgument):
                 self.error(self.usage())
 
     def usage(self):