tox icon indicating copy to clipboard operation
tox copied to clipboard

Add option to append overrides

Open ziima opened this issue 2 years ago • 5 comments

What's the problem this feature will solve?

--override option is nice for changes, but currently it only replaces the config file option. But multiple options are list-like and it would be much better to have an option to append to the list without a need to copy the whole contents.

Describe the solution you'd like

I'd like to have an option to provide additions to the config lists, such as deps, passenv, setenv, etc.

Example

I'd like to add ipdb to the testenv to debug the problem like tox --override testenv.deps=ipdb run -e py310, but it also drops all the deps from the testenv config and tests crashes completely unless I explicitly set deps to all current deps and the ipdb.

Also I'd like to be able to pass such option to user level config so I can have it in all testenvs.

Alternative Solutions

I have working plugin https://pypi.org/project/tox-ipdb-plugin/ for the tox3, but it seems it can almost be replaced only by a user config. And separate plugin would be needed if other changes would be needed.

ziima avatar Dec 16 '22 15:12 ziima

Appending only makes sense here because this is a list, but how would appending work for a boolean flag? 😆 I don't think you can generalize this 🤔 For what's worth the legacy https://tox.wiki/en/latest/cli_interface.html#tox-legacy---force-dep is what you likely want 😊 fixed by unreleased https://github.com/tox-dev/tox/pull/2731; perhaps there's a case to make this part of the non-legacy endpoints 😊

gaborbernat avatar Dec 16 '22 16:12 gaborbernat

how would appending work for a boolean flag?

Indeed, that wouldn't work and I am OK with that :-) I'm not suggesting that override should always append, but that there should be an option to append upon request.

E.g. --override testenv.deps=ipdb would override the deps, but --override testenv.deps+=ipdb would append to it.

--force-dep would probably work, but I'd like to have a future-proof solution :smile:

ziima avatar Dec 16 '22 16:12 ziima

That syntax is not supported by argparse, so supporting that sounds like you're asking a lot of heavy lifting 😅

gaborbernat avatar Dec 16 '22 17:12 gaborbernat

I'm looking at what it would take to do this, and it looks like the current parse is done with .partition("=").

So I'm wondering, could we do something as simple as checking for a key with +?

check endswith("+")
--- a/src/tox/config/loader/api.py
+++ b/src/tox/config/loader/api.py
@@ -26,18 +26,26 @@ class Override:
         key, equal, self.value = value.partition("=")
         if not equal:
             raise ArgumentTypeError(f"override {value} has no = sign in it")
+        self.append = key.endswith("+")
+        if self.append:
+            key = key[:-1]
         self.namespace, _, self.key = key.rpartition(".")

(__str__ and __eq__ get updates too)


Another possible solution would be to add another option which is dedicated to doing an append.

adding an --append option
--- a/src/tox/config/loader/api.py
+++ b/src/tox/config/loader/api.py
@@ -162,6 +170,14 @@ def tox_add_option(parser: ToxParser) -> None:
         dest="override",
         help="configuration override(s)",
     )
+    parser.add_argument(
+        "--append",
+        action="append",
+        type=Override,
+        default=[],
+        dest="append",
+        help="append values to existing config (lists only)",
+    )


Both of these proposals are just about the parsing side of things. I haven't gotten into looking at where and how overrides are applied (which seems to me like the harder part, but maybe I'm wrong! 😅 ).

sirosen avatar Jan 03 '23 01:01 sirosen

This is all a parsing issue, the override being applied is already resolved.

gaborbernat avatar Jan 03 '23 16:01 gaborbernat