Allow only one `--filter` argument and simplify code

Signed-off-by: Svyatoslav Ilinskiy <ilinskiy.sv@gmail.com>

compose/cli/main.py

@@ -599,47 +599,51 @@ class TopLevelCommand(object):
         """
         List containers.
 
-        Usage: ps [options] [--filter KEY=VAL...] [SERVICE...]
+        Usage: ps [options] [--filter KEY=VAL] [SERVICE...]
 
         Options:
             -q                   Only display IDs
             --services           Display services
-            --filter KEY=VAL     Filter services by a property (can be used multiple times)
+            --filter KEY=VAL     Filter services by a property
         """
-        if options['--services']:
+        if options['-q'] and options['--services']:
-            filters = build_filters(options.get('--filter'))
+            raise UserError('-q and --services cannot be combined')
-            services = self.project.services
-            if filters:
-                services = filter_services(filters, services, self.project)
-            print('\n'.join(service.name for service in services))
-        else:
-            containers = sorted(
-                self.project.containers(service_names=options['SERVICE'], stopped=True) +
-                self.project.containers(service_names=options['SERVICE'], one_off=OneOffFilter.only),
-                key=attrgetter('name'))
 
-            if options['-q']:
+        if options['--services']:
-                for container in containers:
+            filt = build_filter(options.get('--filter'))
-                    print(container.id)
+            services = self.project.services
-            else:
+            if filt:
-                headers = [
+                services = filter_services(filt, services, self.project)
-                    'Name',
+            print('\n'.join(service.name for service in services))
-                    'Command',
+            return
-                    'State',
+
-                    'Ports',
+        containers = sorted(
-                ]
+            self.project.containers(service_names=options['SERVICE'], stopped=True) +
-                rows = []
+            self.project.containers(service_names=options['SERVICE'], one_off=OneOffFilter.only),
-                for container in containers:
+            key=attrgetter('name'))
-                    command = container.human_readable_command
+
-                    if len(command) > 30:
+        if options['-q']:
-                        command = '%s ...' % command[:26]
+            for container in containers:
-                    rows.append([
+                print(container.id)
-                        container.name,
+        else:
-                        command,
+            headers = [
-                        container.human_readable_state,
+                'Name',
-                        container.human_readable_ports,
+                'Command',
-                    ])
+                'State',
-                print(Formatter().table(headers, rows))
+                'Ports',
+            ]
+            rows = []
+            for container in containers:
+                command = container.human_readable_command
+                if len(command) > 30:
+                    command = '%s ...' % command[:26]
+                rows.append([
+                    container.name,
+                    command,
+                    container.human_readable_state,
+                    container.human_readable_ports,
+                ])
+            print(Formatter().table(headers, rows))
 
     def pull(self, options):
         """
@@ -1324,34 +1328,34 @@ def build_exec_command(options, container_id, command):
 
 
 def has_container_with_state(containers, state):
+    states = {
+        'running': lambda c: c.is_running,
+        'stopped': lambda c: not c.is_running,
+        'paused': lambda c: c.is_paused,
+        'restarting': lambda c: c.is_restarting,
+    }
     for container in containers:
-        states = {
-            'running': container.is_running,
-            'stopped': not container.is_running,
-            'paused': container.is_paused,
-        }
         if state not in states:
             raise UserError("Invalid state: %s" % state)
-        if states[state]:
+        if states[state](container):
             return True
-    return False
 
 
-def filter_services(filters, services, project):
+def filter_services(filt, services, project):
     def should_include(service):
-        for f in filters:
+        for f in filt:
             if f == 'status':
+                state = filt[f]
                 containers = project.containers([service.name], stopped=True)
-                for status in filters[f]:
+                if not has_container_with_state(containers, state):
-                    if not has_container_with_state(containers, status):
+                    return False
-                        return False
             elif f == 'key':
-                for key in filters[f]:
+                key = filt[f]
-                    if key == 'image' or key == 'build':
+                if key == 'image' or key == 'build':
-                        if key not in service.options:
+                    if key not in service.options:
-                            return False
+                        return False
-                    else:
+                else:
-                        raise UserError("Invalid option: %s" % key)
+                    raise UserError("Invalid value for key filter: %s" % key)
             else:
                 raise UserError("Invalid filter: %s" % f)
         return True
@@ -1359,13 +1363,11 @@ def filter_services(filters, services, project):
     return filter(should_include, services)
 
 
-def build_filters(args):
+def build_filter(arg):
-    filters = {}
+    filt = {}
-    for arg in args:
+    if arg is not None:
         if '=' not in arg:
             raise UserError("Arguments to --filter should be in form KEY=VAL")
         key, val = arg.split('=', 1)
-        if key not in filters:
+        filt[key] = val
-            filters[key] = []
+    return filt
-        filters[key].append(val)
-    return filters

tests/acceptance/cli_test.py

@@ -473,12 +473,12 @@ class CLITestCase(DockerClientTestCase):
         build = self.dispatch(['ps', '--services', '--filter', 'key=build'])
         all_services = self.dispatch(['ps', '--services'])
 
-        self.assertIn('with_build', all_services.stdout)
+        assert 'with_build' in all_services.stdout
-        self.assertIn('with_image', all_services.stdout)
+        assert 'with_image' in all_services.stdout
-        self.assertIn('with_build', build.stdout)
+        assert 'with_build' in build.stdout
-        self.assertNotIn('with_build', image.stdout)
+        assert 'with_build' not in image.stdout
-        self.assertIn('with_image', image.stdout)
+        assert 'with_image' in image.stdout
-        self.assertNotIn('with_image', build.stdout)
+        assert 'with_image' not in build.stdout
 
     def test_ps_services_filter_status(self):
         self.base_dir = 'tests/fixtures/ps-services-filter'
@@ -486,15 +486,14 @@ class CLITestCase(DockerClientTestCase):
         self.dispatch(['pause', 'with_image'])
         paused = self.dispatch(['ps', '--services', '--filter', 'status=paused'])
         stopped = self.dispatch(['ps', '--services', '--filter', 'status=stopped'])
-        running = self.dispatch(['ps', '--services', '--filter', 'status=running',
+        running = self.dispatch(['ps', '--services', '--filter', 'status=running'])
-                                 '--filter', 'key=build'])
 
-        self.assertNotIn('with_build', stopped.stdout)
+        assert 'with_build' not in stopped.stdout
-        self.assertNotIn('with_image', stopped.stdout)
+        assert 'with_image' not in stopped.stdout
-        self.assertNotIn('with_build', paused.stdout)
+        assert 'with_build' not in paused.stdout
-        self.assertIn('with_image', paused.stdout)
+        assert 'with_image' in paused.stdout
-        self.assertIn('with_build', running.stdout)
+        assert 'with_build' in running.stdout
-        self.assertNotIn('with_image', running.stdout)
+        assert 'with_image' in running.stdout
 
     def test_pull(self):
         result = self.dispatch(['pull'])