Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

More robustness for getting os version on Windows #159

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

More robustness for getting os version on Windows #159

wants to merge 3 commits into from

Conversation

Joouis
Copy link

@Joouis Joouis commented May 28, 2017

Description of the Change

On Windows OS, if the system language is not English, type systeminfo in CMD would get "OS" with native chars instead of "OS Name". Change the regex statement to match "OS" with "Microsoft", so that winVersionText function can return useful version info on non-English system as well as English system.

Alternate Designs

None.

Benefits

Get useful system version on non-English Windows OS instead of "Unknown Windows version".

Possible Drawbacks

None.

Applicable Issues

None.

@Joouis
Copy link
Author

Joouis commented May 28, 2017

Why AppVeyor build failed 😥 I just change a regex statement, and in ENV ATOM_CHANNEL=stable passed but in beta failed?😵😵😵

@damieng
Copy link
Contributor

damieng commented May 31, 2017

I think we might be better off just using os.release() as it gives a more complete version number (knowing it is Pro isn't much use to us). This would also work on non-English locales.

@Ben3eeE
Copy link

Ben3eeE commented Jul 5, 2017

Calling systeminfo can be very slow. On one of the computers I use it can take up to 60 seconds to complete being stuck at loading hotfix information for a very long time, no idea why it is so slow.

@Joouis
Copy link
Author

Joouis commented Jul 6, 2017

@Ben3eeE You are right, calling systeminfo consumes a long time, and make it an async event. :(

@damieng
Copy link
Contributor

damieng commented Jul 6, 2017

As the primary Windows guy I've not once found Microsoft Windows 10 Pro more useful than 10.0.15063. The latter provides a lot more info and the former only potentially has value in the "Pro" designation - but I don't think we've ever had a bug in Atom that was in any way related to the edition of Windows. It's not even in our build and test matrix.

@winstliu
Copy link
Contributor

winstliu commented May 1, 2018

If we keep using systeminfo, the proposed regex still won't work for Spanish, which doesn't use the OS abbreviation.

I like os.release() as well.

UziTech added a commit to UziTech/notifications that referenced this pull request Sep 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants