- Fixed behavior of bool --X/--disable-X

- Fixed beahvore of defaults with `store_or_None`
- Fixed checking defaults vs list/tuple instead of flattened value
- Added checks for list values in covert comming from the defaults

Author: Crivella

easybuild/cli/__init__.py

@@ -31,34 +31,61 @@ def eb(ctx, other_args):
     for key, value in getattr(ctx, 'hidden_params', {}).items():
         key = key.replace('_', '-')
         opt = EasyBuildCliOption.OPTIONS_MAP[key]
+        # TEST_KEYS = [
+        #     'robot',
+        #     'robot-paths',
+        #     'map-toolchains',
+        #     'search-paths',
+        # ]
+        # if key in TEST_KEYS:
+        #     print(f'{key.upper()}: `{value=}` `{type(value)=}` `{opt.default=}` `{opt.action=}`')
         if value in ['False', 'True']:
             value = value == 'True'
         if isinstance(value, bool):
-            if value:
-                args.append(f"--{key}")
+            if value != opt.default:
+                if value:
+                    args.append(f"--{key}")
+                else:
+                    args.append(f"--disable-{key}")
         else:
+
             if isinstance(value, (list, tuple)) and value:
                 # Flatten nested lists if necessary
                 if isinstance(value[0], list):
                     value = sum(value, [])
             # Match the type of the option with the default to see if we need to add it
-            if isinstance(value, list) and isinstance(opt.default, tuple):
+            if value and isinstance(value, list) and isinstance(opt.default, tuple):
                 value = tuple(value)
-            if value and value != opt.default:
-                if isinstance(value, (list, tuple)):
-                    if 'path' in opt.type:
-                        delim = os.pathsep
-                    elif 'str' in opt.type:
-                        delim = ','
-                    elif 'url' in opt.type:
-                        delim = '|'
-                    else:
-                        raise ValueError(f"Unsupported type for {key}: {opt.type}")
-                    value = delim.join(value)
+            if value and isinstance(value, tuple) and isinstance(opt.default, list):
+                value = list(value)
+            value_is_default = (value == opt.default)
 
-                # print(f"--Adding {key}={value} to args")
-                args.append(f"--{key}={value}")
+            value_flattened = value
+            if isinstance(value, (list, tuple)):
+                if 'path' in opt.type:
+                    delim = os.pathsep
+                elif 'str' in opt.type:
+                    delim = ','
+                elif 'url' in opt.type:
+                    delim = '|'
+                else:
+                    raise ValueError(f"Unsupported type for {key}: {opt.type}")
+                value_flattened = delim.join(value)
 
-    args.extend(other_args)
+            if opt.action == 'store_or_None':
+                if value is None or value == ():
+                    continue
+                if value_is_default:
+                    args.append(f"--{key}")
+                else:
+                    args.append(f"--{key}={value_flattened}")
+            elif value and not value_is_default:
+                if value:
+                    args.append(f"--{key}={value_flattened}")
+                else:
+                    args.append(f"--{key}")
+    # for arg in args:
+    #     print(f"ARG: {arg}")
 
+    args.extend(other_args)
     main_with_hooks(args=args)

easybuild/cli/options/__init__.py

@@ -42,11 +42,16 @@ def __init__(self, *args, delimiter=',', resolve_full: bool = True, **kwargs):
         self.resolve_full = resolve_full
 
     def convert(self, value, param, ctx):
-        if not isinstance(value, str):
+        # logging.warning(f"{param=} convert called with `{value=}`, `{type(value)=}`")
+        if isinstance(value, str):
+            res = value.split(self.delimiter)
+        elif isinstance(value, (list, tuple)):
+            res = value
+        else:
             raise click.BadParameter(f"Expected a comma-separated string, got {value}")
-        res = value.split(self.delimiter)
         if self.resolve_full:
             res = [os.path.abspath(v) for v in res]
+        # logging.warning(f"{param=} convert returning `{res=}`")
         return res
 
     def shell_complete(self, ctx, param, incomplete):
@@ -83,8 +88,12 @@ def __init__(self, *args, delimiter=',', **kwargs):
 
     def convert(self, value, param, ctx):
         if isinstance(value, str):
-            return value.split(self.delimiter)
-        raise click.BadParameter(f"Expected a string or a comma-separated string, got {value}")
+            res = value.split(self.delimiter)
+        elif isinstance(value, (list, tuple)):
+            res = value
+        else:
+            raise click.BadParameter(f"Expected a string or a comma-separated string, got {value}")
+        return res
 
     def shell_complete(self, ctx, param, incomplete):
         last = incomplete.rsplit(self.delimiter, 1)[-1]
@@ -124,13 +133,15 @@ def __post_init__(self):
 
     def to_click_option_dec(self):
         """Convert OptionData to a click.Option."""
-        decls = [f"--{self.name}"]
+        decl = f"--{self.name}"
+        other_decls = []
         if self.short:
-            decls.insert(0, f"-{self.short}")
+            other_decls.insert(0, f"-{self.short}")
 
         kwargs = {
             'help': self.description,
             'default': self.default,
+            'is_flag': False,
             'show_default': True,
             'type': None
         }
@@ -139,7 +150,6 @@ def to_click_option_dec(self):
             kwargs['type'] = DelimitedString(delimiter=',')
             kwargs['multiple'] = True
         elif self.type in ['pathlist', 'pathtuple']:
-            # kwargs['type'] = DelimitedPathList(delimiter=os.pathsep)
             kwargs['type'] = DelimitedPathList(delimiter=',')
             kwargs['multiple'] = True
         elif self.type in ['urllist', 'urltuple']:
@@ -148,7 +158,7 @@ def to_click_option_dec(self):
         elif self.type == 'choice':
             if self.lst is None:
                 raise ValueError(f"Choice type requires a list of choices for option {self.name}")
-            kwargs['type'] = click.Choice(self.lst, case_sensitive=False)
+            kwargs['type'] = click.Choice(self.lst, case_sensitive=True)
         elif self.type in ['int', int]:
             kwargs['type'] = click.INT
         elif self.type in ['float', float]:
@@ -159,10 +169,18 @@ def to_click_option_dec(self):
             if self.default is False or self.default is True:
                 kwargs['is_flag'] = True
                 kwargs['type'] = click.BOOL
+                if self.action in ['store_true', 'store_false']:
+                    decl = f"--{self.name}/--disable-{self.name}"
             elif isinstance(self.default, (list, tuple)):
                 kwargs['multiple'] = True
                 kwargs['type'] = click.STRING
 
+        if self.action == 'store_or_None':
+            kwargs['default'] = None
+            kwargs['flag_value'] = self.default
+
+        decls = other_decls + [decl]
+
         return click.option(
             *decls,
             expose_value=False,