From 556a7eff9d3bc83659d0e8fbccf18c5e2f852173 Mon Sep 17 00:00:00 2001
From: l_samenv <l_samenv@samenv.psi.ch>
Date: Tue, 21 Jan 2025 10:49:54 +0100
Subject: [PATCH] improve missing arg handling

- better error messages
- missing instrument is only allowed with 'list'
- missing argument to be replaced with ? in error message
- sea cli may now be given with 'graph' or 'sea' for service
---
 __init__.py | 11 +++++++----
 base.py     | 20 +++++++++++---------
 seaman.py   |  7 +++++--
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/__init__.py b/__init__.py
index c17b2b5..d5e7461 100644
--- a/__init__.py
+++ b/__init__.py
@@ -49,7 +49,7 @@ class SewebManager(ServiceManager):
 
 
 all = NicosManager, FrappyManager, SeaManager, SewebManager
-
+KINDS = 'action', 'ins', 'service'
 
 def run(group, arglist):
     try:
@@ -68,7 +68,7 @@ def run(group, arglist):
            'service': lambda arg: arg in serv.services,
         }
 
-        for kind in 'action', 'ins', 'service':
+        for kind in KINDS:
             if arglist and arg_is[kind](arglist[0]):  # arg is of expected kind
                 args[kind] = arglist.pop(0)
                 continue
@@ -91,8 +91,11 @@ def run(group, arglist):
             args.setdefault('ins', serv.main_ins)
         if guessed_args:
             args.setdefault('action', 'gui')
-            print('do you mean:\n   %s %s %s %s %s' %
-                  (group, args.get('action', ''), args.get('ins', ''), args.get('service', ''), ' '.join(extra)))
+            guessed = [group] + [args.get(k, '') for k in KINDS] + extra
+            while not guessed[-1]:
+                guessed.pop()
+            guessed = ' '.join(a or '?' for a in guessed)
+            print(f'do you mean:\n   {guessed}')
         else:
             try:
                 serv.action(args['action'], *serv.treat_args(args, extra + arglist))
diff --git a/base.py b/base.py
index 8144dee..3bb027c 100644
--- a/base.py
+++ b/base.py
@@ -222,7 +222,7 @@ class ServiceManager:
 
         or None, when no wildcard character in ins
         """
-        if ins is None or ins == 'all':
+        if not ins or ins == 'all':
             return list(self.info)
         pat = re.sub(r'(\.|\*)', '.*', ins)
         if pat == ins:
@@ -282,9 +282,9 @@ class ServiceManager:
         return done
 
     def do_stop(self, ins, service=None, *args):
+        if not ins:
+            raise UsageError(f'need instrument or "all" to stop all')
         self.get_info()
-        if ins is None:
-            raise ValueError('use stop all if you really want to stop all')
         ins_list = self.wildcard(ins)
         if ins_list is not None:
             return ins_list
@@ -301,6 +301,8 @@ class ServiceManager:
         return env.get('%s_ROOT' % gr, ''), env
 
     def do_start(self, ins, service=None, cfg='', restart=False, wait=False, logger=None, opts=''):
+        if not ins:
+            raise UsageError(f'need instrument or "all" to start all')
         ins_list = self.wildcard(ins)
         if ins_list is not None:
             return ins_list
@@ -319,10 +321,10 @@ class ServiceManager:
             return
         try:
             service_ports = self.get_ins_info(ins)
-        except ValueError:
-            raise ValueError('do not know %r' % ins)
+        except (KeyError, ValueError):
+            raise UsageError('do not know %r' % ins)
         if ins in self.remote_hosts:
-            raise ValueError('can not start, %s is running on a remote host' % self.group)
+            raise UsageError('can not start, %s is running on a remote host' % self.group)
         services = list(service_ports) if service is None else [service]
         if restart:
             self.stop(ins, service)
@@ -406,8 +408,8 @@ class ServiceManager:
                 os.chdir(wd)
 
     def do_restart(self, ins, service=None, cfg=None, logger=None):
-        if ins is None:
-            raise UsageError("need instance or 'all' or wildcard")
+        if not ins:
+            raise UsageError("need instrument or 'all' or wildcard")
         ins_list = self.wildcard(ins)
         if ins_list is not None:
             if cfg is not None:
@@ -418,7 +420,7 @@ class ServiceManager:
     def do_run(self, ins, service=None, cfg=None, opts=''):
         """for tests: run and wait"""
         if self.wildcard(ins) is not None:
-            raise UsageError('no wildcards allowed with %s run' % self.group)
+            raise UsageError('need instrument and service for "%s run"' % self.group)
         if not service:
             try:
                 service, = self.services
diff --git a/seaman.py b/seaman.py
index 2ba7501..2e100e3 100644
--- a/seaman.py
+++ b/seaman.py
@@ -68,7 +68,7 @@ class SeaManager(ServiceManager):
 %(legend)s
     """
 
-    def do_cli(self, ins):
+    def do_cli(self, ins, service=None):
         if self.wildcard(ins):
             raise UsageError('wildcards not allowed in sea cli')
         try:
@@ -78,7 +78,8 @@ class SeaManager(ServiceManager):
             print(str(e))
         except KeyError:  # running on an other machine?
             self.do_help()
-        run_command('six -sea %s' % ins, wait=True)
+        service = service or 'sea'
+        run_command(f'six -{service} {ins}', wait=True)
 
     def do_gui(self, ins='', *args):
         if ins and self.wildcard(ins):
@@ -196,6 +197,8 @@ class SeaManager(ServiceManager):
                     argdict['ins'] = arg
                 else:
                     raise UsageError('unknown argument: %s' % arg)
+            else:
+                print('env variable "InstrumentHostList" is not defined')
         result = [argdict.pop('ins', '')]
         service = argdict.pop('service', '')
         if service: