HipsterBrown / xs-dev

The quickest way for getting started with JS on devices
https://xs-dev.js.org
MIT License
41 stars 13 forks source link

feat: Windows setup extension start (#23) #44

Closed andycarle closed 2 years ago

andycarle commented 2 years ago

Start of Windows setup extension.

To-Do:

andycarle commented 2 years ago

@HipsterBrown Thank you for the thoughtful review! This is my first foray into non-trivial TypeScript, so I'm learning a lot here.

I'll take a pass on this today and see what I come up with.

andycarle commented 2 years ago

Hi @HipsterBrown!

I'm hoping to prioritize Windows support for xs-dev over the next couple of weeks. The new commits above (which of course would need to be squashed down... a lot... before merging) are a start. On my system, xs-dev setup and xs-dev update are now working well.

There may still need to be some updates to the defaults in constants.ts to make it make a bit more sense on Windows. But I am not sufficiently confident in where people prefer tools like this to reside on Windows to make the change right now. I'll do a bit of research there.

I'm going to take a crack at initial ESP8266 and ESP32 support next.

HipsterBrown commented 2 years ago

On my system, xs-dev setup and xs-dev update are now working well.

@andycarle that's great! To keep the amount of changes per pull request manageable, how do you feel about splitting the ESP8266 and ESP32 work into separate PRs?

Once we get the build checks complete (looks like I need to update those workflow files on main), then I'm open to approving and merging this set of changes in.

andycarle commented 2 years ago

Yes, I agree that we can split the ESP8266 and ESP32 work out to another PR. I'm getting started on those today.

I'm also double-checking some things around PATH management in this PR. I'm concerned about the possibility of the PATH being corrupted in some edge cases—I'll get that sorted out.

andycarle commented 2 years ago

I sorted out the PATH management issue that I was worried about above.

The gist of issue is that Windows registry keys are case-insensitive, but the registry does store the case chosen by whatever most recently set the registry key. And the regedit npm package faithfully uses the stored case as the name of the key in the corresponding JavaScript object which, of course, is case sensitive. So, to find the existing PATH you need to do a case-insensitive search through the object's keys.

The newly pushed version does just that. I acknowledge an arguably more elegant way of managing that object with a Proxy, but I decided that this approach would suffice. 😆

andycarle commented 2 years ago

This PR is superseded by #53, which takes a cleaner approach to environment management.