Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to append overrides #2738

Open
ziima opened this issue Dec 16, 2022 · 5 comments
Open

Add option to append overrides #2738

ziima opened this issue Dec 16, 2022 · 5 comments
Labels
enhancement help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@ziima
Copy link
Contributor

ziima commented Dec 16, 2022

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.

@gaborbernat
Copy link
Member

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 #2731; perhaps there's a case to make this part of the non-legacy endpoints 😊

@gaborbernat gaborbernat added the help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. label Dec 16, 2022
@ziima
Copy link
Contributor Author

ziima commented Dec 16, 2022

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 😄

@gaborbernat
Copy link
Member

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

@gaborbernat gaborbernat changed the title Add opton to append overrides Add option to append overrides Dec 17, 2022
@gaborbernat gaborbernat added this to the 4.1 milestone Dec 17, 2022
@sirosen
Copy link
Contributor

sirosen commented Jan 3, 2023

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! 😅 ).

@gaborbernat
Copy link
Member

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

@gaborbernat gaborbernat removed this from the P-1 milestone Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

No branches or pull requests

3 participants