-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Applied changes by nicomen to prevent "VAR will not stay shared" messages #709
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a good fix. It was just a test to see if the theory was correct. The various "shared variables" should be passed in as parameters were fitting in the various functions.
Also there are a few more shared variable errors in the modules.
I would wait to merge this and see if I can make a slightly nicer fix.
Sure, shall we close this PR then? |
Yes, please. I'll send a less destructive PR shortly. |
So, I went thru the code variable by variable trying to see which ones had to be global, and which one didn't. That took way too long and I didn't finish. But the result is that most variables need to global the way the code is written, so this patch is not THAT bad after all. Oh well. |
Thanks for taking so much time to deal with it! |
It's baffling that this has ever worked if it always was like that. Perhaps there is something I'm not getting. But oh well. Probably some more intelligent persons than me know some best practices in how to keep around state variables and such in a cleaner fashion when doing GUI programming. |
It was different during Gtk2 times. Slightly different. I made it like this during migration to Gtk3, because I don't know any better. I only made it work without caring too much about code quality, which was not the best already... |
Aha, that makes much more sense! Should probably have looked at the history :-) Thanks for getting the opprtunity to help. I found a new tool that I can use :-) |
So, should we merge this PR? |
See #703 (comment)
Should close #338.
Thanks @nicomen!