Skip to content

fix: Do not create command prompt window on subprocess #436

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

Merged
merged 5 commits into from
Sep 3, 2024

Conversation

seakrueger
Copy link
Collaborator

Patches files from abandoned libraries are located and updated in src/qt/helpers/vendored with modified sections labeled PATCHED. A wrapper around subprocess.Popen automatically sets the creation flag to no window on windows.

Must be run via a pyinstaller executable for problem to occur.

@CyanVoxel CyanVoxel added Type: Bug Something isn't working as intended Priority: Critical An issue that requires immediate attention Status: Review Needed A review of this is needed Status: Help Wanted Extra attention is needed labels Sep 3, 2024
@CyanVoxel CyanVoxel added this to the Alpha v9.4 (Pre-SQL) milestone Sep 3, 2024
@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Sep 3, 2024
@seakrueger seakrueger force-pushed the silent-popen branch 2 times, most recently from f076cdc to b5b2ccf Compare September 3, 2024 02:44
Comment on lines +2 to +9
exclude = ["main_window.py", "home_ui.py", "resources.py", "resources_rc.py", "**/vendored/"]

[tool.mypy]
strict_optional = false
disable_error_code = ["union-attr", "annotation-unchecked", "import-untyped"]
explicit_package_bases = true
warn_unused_ignores = true
exclude = ['tests']
exclude = ['tests', 'src/qt/helpers/vendored']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why the ruff exclusion pattern for vendored is different than mypy's?

@CyanVoxel CyanVoxel added the Status: Review Needed A review of this is needed label Sep 3, 2024
@CyanVoxel
Copy link
Member

This seems to be working when building on Windows with PyInstaller 6.10.0 and testing with video previews and audio waveform previews. I don't see anything immediately wrong with this besides MyPy complaining about the vendored files, which might not be excluded correctly. Also for some reason the last 4 commits keep creeping forward since they say they were authored in the future 🤷‍♂️

@CyanVoxel CyanVoxel removed Status: Help Wanted Extra attention is needed Status: Review Needed A review of this is needed labels Sep 3, 2024
@CyanVoxel
Copy link
Member

Pulling this now with some ignore comments on the affected files. Couldn't seem to get the folder to exclude either, but this will work for now. Thank you so much for getting a fix for this!

@CyanVoxel CyanVoxel merged commit fc714e0 into TagStudioDev:Alpha-v9.4 Sep 3, 2024
4 checks passed
@CyanVoxel CyanVoxel removed the Priority: Critical An issue that requires immediate attention label Sep 3, 2024
seakrueger and others added 5 commits September 3, 2024 01:40
Patches files from abandoned libraries are located and updated in
src/qt/helpers/vendored with modified sections labeld PATCHED. A wrapper
around subprocess.Popen automatically sets the creation flag to no
window on windows.
CarterPillow pushed a commit to CarterPillow/TagStudio that referenced this pull request Sep 7, 2024
)

* fix: Do not create command prompt window on subcmd

Patches files from abandoned libraries are located and updated in
src/qt/helpers/vendored with modified sections labeld PATCHED. A wrapper
around subprocess.Popen automatically sets the creation flag to no
window on windows.

* fix: Replace Popen in mediainfo_json decoder

* fixup: Pipe stdin to stdin

* chore: Exclude vendored dir from tooling checks

* suppress mypy warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants