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

Miscellaneous path configuration issues #346

Conversation

Versatilus
Copy link
Collaborator

The path to the Python interpreter used to run asynchronous commands such as the custom MouseGrid windows and settings window has been determined by a variable stored in the main configuration file. The past default value was manually set to the stereotypically default Python installation path. This resulted in a failure, usually silently, of these asynchronous commands if the user used a different path for their Python installation.

My solution for this problem uses both the sys and sysconfig modules to build a database of relevant system information, one which can be easily expand upon in the future. Using this information, the default value of that variable is automatically set to the location of the installed PythonW executable.

It was around this time that we noticed that the purpose of the variable pointing to the current wxPython distribution was an effort to get around a weird installation issue. This is more appropriately fixed by upgrading the installed version of wxPython. The configuration variable was removed and an appropriate error message was added to the settings window script, which was the only section of code that uses wxPython.

In the process of fixing this issue a generic message directing users to places they can get help was added to the settings module for future use.

Addressing these issues resulted in the following commits:

  • Automatically detect pythonw.exe path if available.
  • Minor changes to improve future Python 3 compatibility.
  • Added a generic help message for use in error messages.
  • Require the latest version of wxPython if the module import fails
    instead of using a configuration variable to work around an old wxPython
    installation issue.

Minor changes to improve future Python 3 compatibility

Added a generic help message for use in error messages.

Require the latest version of wxPython if the module import fails
instead of using a configuration variable to work around an old wxPython
installation issue.
@LexiconCode LexiconCode added the Enhancement Enhancement of an existing feature label Dec 14, 2018
@LexiconCode
Copy link
Member

LexiconCode commented Dec 15, 2018

Regarding PYTHONW in the settings. Should the path PYTHONW = "C:\\Python27\\pythonw.exe" be PYTHONW = "C:/Python27/pythonw.exe"?

replace( "\\", "/")

@Versatilus
Copy link
Collaborator Author

As best as I can tell, they should really all be done with backslashes instead of forward slashes. I believe the reason it was done this way is because escaping the backslash character in sequences is easier for people and using the forward slash character generally works most of the time. Because everything else in the Windows ecosystem expects the directory separator character to be the backslash, it can cause issues when interacting with system functions.

I've been thinking lately that it would be a good idea to at least transparently pass paths to Windows in the expected form. This can easily be done using Python's os.path.normpath() function.

@Versatilus
Copy link
Collaborator Author

If no one has any objections, I'll be merging this in the morning (15:00-19:00 GMT Thursday).

More robust path handling, allowing "caster" further up the directory tree

Co-Authored-By: Versatilus <[email protected]>
@mrob95
Copy link
Member

mrob95 commented Dec 20, 2018

That same line exists in quite a few places, I think I changed them all in #336
synkarius@c51d7f5

@Versatilus
Copy link
Collaborator Author

I'll look that over in a few hours before I decide what to do with this. I got distracted this week and I never got back to it.

@LexiconCode
Copy link
Member

LexiconCode commented May 4, 2019

Can we update pull request to prep for merge?

@kendonB
Copy link
Collaborator

kendonB commented May 18, 2019

I have updated this PR with the latest state of the repo. It might be worth one final review from @Versatilus or @LexiconCode

kendonB added a commit to kendonB/Caster that referenced this pull request May 18, 2019
@LexiconCode
Copy link
Member

I will get this tested and merged Monday! Thanks for bringing it up-to-date!

@LexiconCode
Copy link
Member

Works as expected. @kendonB

As best as I can tell, they should really all be done with backslashes instead of forward slashes. I believe the reason it was done this way is because escaping the backslash character in sequences is easier for people and using the forward slash character generally works most of the time. Because everything else in the Windows ecosystem expects the directory separator character to be the backslash, it can cause issues when interacting with system functions.

I've been thinking lately that it would be a good idea to at least transparently pass paths to Windows in the expected form. This can easily be done using Python's os.path.normpath() function.

However normalizing paths using os.path.normpath() needs to be implemented in a separate pull request.

@LexiconCode LexiconCode merged commit 2266132 into dictation-toolbox:develop May 22, 2019
@Versatilus Versatilus deleted the pr-Python-path-improvements-20181209 branch January 4, 2020 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Enhancement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants