Skip to content

Comments

Allow optional arguments for running tox#9

Merged
The-Compiler merged 8 commits intoThe-Compiler:mainfrom
The-V8:feature/add-tox-arguments
Jun 23, 2022
Merged

Allow optional arguments for running tox#9
The-Compiler merged 8 commits intoThe-Compiler:mainfrom
The-V8:feature/add-tox-arguments

Conversation

@pkirch
Copy link
Contributor

@pkirch pkirch commented Apr 7, 2022

Change the flow for running tox python-tox.select to accept optional arguments for tox, e.g. parameters like -vv or -- {posargs}.

Changed function runTox to use dependency injection to get the vscode.Terminal to be able to test this function.
Add a unit test to verify the correct usage of the additional arguments using a mocked vscode.Terminal.

This PR sets the foundation for the enhancement mentioned in #2 and realizes #3.

starlord-daniel and others added 7 commits April 5, 2022 12:01
Co-authored-by: Peter Kirchner <pkirch@users.noreply.github.com>
Use dependency injection to make function runTox testable.

Co-authored-by: Oliver L <se02035@users.noreply.github.com>
@The-Compiler
Copy link
Owner

FYI, I'll mostly be away until after Easter (at PyConDE and then a digital arts event) - thus, I'll probably only get to reviewing this in 2 weeks or so. Meanwhile, thanks for the contribution and your patience!

@The-Compiler The-Compiler merged commit c69fd5f into The-Compiler:main Jun 23, 2022
@The-Compiler
Copy link
Owner

One down, six to go - thanks @pkirch!

I pushed some small changes/improvements on top:

  • b04b50d Add separate run(Multiple)WithArgs commands
  • 8ab3f49 Remove debug logging
  • ebd18c5 Improve argument input box

I felt like it was strange to always ask for arguments with select but never with selectMultiple - I can see both being useful in both circumstances. Thus, I instead opted to keep the existing commands as-is, but add new selectWithArgs and selectMultipleWithArgs commands instead.

I'm also not sure what's up with the big package-lock.json change - I suppose that came from you running npm install after adding ts-mockito and perhaps having an older npm version than I do, as I got this with the next npm install:

npm WARN old lockfile 
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile 
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile 

but I suppose after I regenerated it in ab9766a, that won't matter much anymore. In any case, thanks for taking care of adding a test as well, much appreciated!

@pkirch
Copy link
Contributor Author

pkirch commented Jun 23, 2022

@The-Compiler Thanks for working through the PRs! I really appreciate your thoroughness in understanding the changes and aligning with your vision of the extension!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants