gpduck / MSTerminalSettings

MIT License
118 stars 15 forks source link

Cannot manage Development build with Store version present #31

Closed jpogran closed 5 years ago

jpogran commented 5 years ago

Awesome work!

I've used this module to successfully manage the Store version of the Windows Terminal settings, but I've run into an issue where I can't manage the development version if both are installed on the same system.

The following code is the cause:

https://github.com/gpduck/MSTerminalSettings/blob/3a17a2f97bd7ce3b8f2a764d02992dc56fb64152/src/ExportedFunctions/Find-MSTerminalFolder.ps1#L13-L24

The array lists the Store version first, then in the foreach loop it breaks if it finds a valid path. Since the development version is last, on a system with both present, it will never be 'found'.

At first I thought I could do a quick PR and modify Find-MSTerminalFolder to have a switch like $devBuild or something similar, but that would mean modifying the call sites for all other functions that call this, which is quite a few. So I thought then I should raise an issue and figure out what direction you would want to take, then offer my help to get there

gpduck commented 5 years ago

At one point I considered adding a -scope parameter to everything, but now I'm wondering if maybe an environment variable to override the default would be enough?

jpogran commented 5 years ago

I personally would prefer parameters instead of environment variables, but that's a personal bias

gpduck commented 5 years ago

Since the store version has been released I feel like most people are only going to need to target the "real" profile path. I'd like to avoid complicating all the functions for something that's not going to be commonly used.

How about a Set-MSTerminalTargetInstallation -Scope {Dev | Release | Standalone} -Custom $Path that will set a module scoped variable that overrides the search behavior?

jpogran commented 5 years ago

Upon reflection, and considering your recent comment, I think it's a fair point that the store version is the likely primary use case for this module. It's not likely you'll be running from source in a couple months time when this is 'v1', and I also think I may be alone in wanting to manage the dev version side by side with the release version.

I think it's fair to draw a line in the sand today and say 'this is for managing the store version' and remove the paths to the development versions, rather than complicate your code base with a scoped variable.