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

Fixes incorrect replacement usage of IsVista() and IsXP() #221

Merged
merged 1 commit into from Apr 20, 2017

Conversation

kdschlosser
Copy link
Member

The original Utils.IsVista() and Utils.IsXp() would return True ot False if the OS was greater than or eqiual to. We had changed everything to use the new WindowsVersion.IsVista() and IsXP() which would only return True/False if it is that opperating system.

This fixes all the wrongly used IsVista() and IsXP() to use WindowsVersion >= 'Vista' and WindowsVersion >= 'XP'

The original Utils.IsVista() and Utils.IsXp()  would return True ot False if the OS was greater than or eqiual to. We had changed everything to use the new WindowsVersion.IsVista() and IsXP() which would only return True/False if it is that opperating system.

This fixes all the wrongly used IsVista() and IsXP() to use WindowsVersion >= 'Vista' and WindowsVersion >= 'XP'
@kdschlosser
Copy link
Member Author

kdschlosser commented Apr 20, 2017

This is kind of a biggie, and is going to have a huge impact on how EG runs. I would push out another release candidate ASAP. My opinion... ¯\(ツ)

@@ -394,23 +394,23 @@ def IsVista():
"""
warnings.warn(
"eg.Utils.IsVista() is deprecated. "
"Use eg.WindowsVersion.IsVista() instead",
"Use eg.WindowsVersion >= 'Vista' instead",
Copy link
Contributor

Choose a reason for hiding this comment

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

IsVista() means is Vista and not is Vista or later. So i think the message can/should be changed to "Use eg.WindowsVersion.IsVista() or eg.WindowsVersion >= 'Vista' instead."


def IsXP():
"""
Determine if we're running XP or higher.
"""
warnings.warn(
"eg.Utils.IsXP() is deprecated. "
"Use eg.WindowsVersion.IsXP() instead",
"Use eg.WindowsVersion >= 'XP' instead",
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as with IsVista 😉

Copy link
Member Author

Choose a reason for hiding this comment

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

well the original eg.Utils.IsVista() and eg.Utils.IsXP() would return true if the os was >= vista or xp. hence the reason why i only did the >= so this way it will mimic the exact operation of the original functions. where as the new version only returns true if it is that exact os.

if you look at the original code

def IsVista():
    """
    Determine if we're running Vista or higher.
    """
    return (sys.getwindowsversion()[0] >= 6)

def IsXP():
    """
    Determine if we're running XP or higher.
    """
    return (sys.getwindowsversion()[0:2] >= (5, 1))

even the doc string says and higher,

I'll do whatever it is you want me to do on this. but i just wanted to have to depreciation warning show exactly how to use the new to replace the old. no confusion that way

Copy link
Member Author

Choose a reason for hiding this comment

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

let me know what to do ob-wan

Copy link
Contributor

Choose a reason for hiding this comment

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

The return value of eg.Utils.IsVista() is correct. I only meant the deprecation message. Just to make sure that the user/developer notice that there is a difference behaviour of eg.Utils.IsVista() and eg.WindowsVersion.IsVista().
But now while i write this, i think you are right and only display >= 'Vista' because that's how Utils.IsVista() worked.
I will merge it now and make a new release.

Copy link
Member Author

Choose a reason for hiding this comment

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

cool. Thanks for the quick update on this. I am not sure if you want to change up the forum RC1 to inform the users not to use this version that is a large flaw in it. but leave it there for history purposes.

@topic2k topic2k added the bugfix label Apr 20, 2017
@topic2k topic2k changed the title [EventGhost] - Bugfix - WindowsVersion, Fixes incorrect use of IsVista() and IsXP() Fixes incorrect replacement usage of IsVista() and IsXP() Apr 20, 2017
@topic2k topic2k merged commit 8f73109 into EventGhost:master Apr 20, 2017
@kdschlosser kdschlosser deleted the bugfix-windowsversionuse branch April 20, 2017 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants