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

Campaign to Save the CLI #161

Merged
merged 18 commits into from
Oct 7, 2024
Merged

Campaign to Save the CLI #161

merged 18 commits into from
Oct 7, 2024

Conversation

thw26
Copy link
Collaborator

@thw26 thw26 commented Aug 28, 2024

Fixes #128. Requires #158.

I should clarify my aim is to get it working and to make it pretty later (cf. #172).

Part of my goal in this PR is to make #147 potentially easier. The CLI is intentionally aiming to be a highly simplified event/queue manager, thus I aim to clear the event's status frequently in order to reuse it later. The TUI is, to my knowledge, never waiting on more than one event at a time. Therefore we have not need to have multiple events. If we do have some place where we are waiting on multiple, then we can have two different ones at that time. This would make the code much simpler, leaner, and easier to reuse.

What I imagine therefore, from this PR, is extracting all the event/queue code from the TUI, the CLI, and the GUI, into its own class module.

Support the Campaign to Save the CLI
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is fixed in #182

@n8marti
Copy link
Collaborator

n8marti commented Oct 7, 2024

heavy_wineserver_wait() was only being used after running winetricks commands, and after adding debug logging there don't seem to be any processes still using the WINEPREFIX when winetricks commands finish and wineserver_wait() is run. I suggest we keep the debug logging for the time being, that way if anyone has problems b/c of no longer using heavy_wineserver_wait() we'll be able to see if it's due to the WINEPREFIX still being accessed by something.

@thw26 thw26 merged commit a9a00aa into main Oct 7, 2024
@n8marti n8marti deleted the 128-fix-install-optarg branch October 9, 2024 04:18
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.

--install-app Optarg is Broken
2 participants