feat: Store settings in the application folder

[learn-more]

#137

[activescott]

Why this? It adds file I/O logic for one of the things that should be automatic. Also writing such settings into app folder won't work if it is in program files or similar and seems kinda less than ideal to write data into a program directory and instead of a per-user location.

[learn-more]

Why this? It adds file I/O logic for one of the things that should be automatic. Also writing such settings into app folder won't work if it is in program files or similar and seems kinda less than ideal to write data into a program directory and instead of a per-user location.

The issue requested to store it in the application folder, I assumed that is what you meant with 'locally'

As far as I know, the only way to use the application settings is to basically re-write the entire provider anyway, so I found it easier to just write it entirely from scratch. (And probably less code as well.)

To use the existing application settings infra,
one would have to implement a 'SettingsProvider', which also means writing and reading somewhere manually.

[activescott]

Sorry I overlooked the "#137" in the PR description. I like the idea of portable, but now that I saw the PR it did make me think that in a "traditional install" for windows where apps are installed into "Program Files" apps won't be able to write into that directory. We don't actually provide an msi that installs there so it may not be common among lessmsi users. I also checked Chocolatey (which we do provide a popular package for) and apparently Chocolatey does allow writing to the app directory (I did not test though).

Since it is somewhat unorthodox to write data into the same directory as the app though, I wonder if maybe we shouldn't have a fallback to a user-specific, safe location to write settings if we don't have write/create permission or should we just fail and not save settings? What do you think?

[activescott]

Left a couple minor notes on the code. If you could look at those then I'll do a deeper dive and test this one out and as long as I don't run into any obvious errors I'll merge.

Please do make sure that you have tested it. If you could please test in c:\Program Data... like chocolatey uses, and in c:\Program Files...\ (where it should fail gracefully) and confirm I will really appreciate it!

Thanks again for contributing!

[activescott]

I have been bit by these catch handlers without named exceptions in the past. Can you please catch the expected exception from this list here (maybe just UnauthorizedAccessException or SecurityException or something like that)? Let the other ones that you never expect to happen bubble up. I think the unhandled exception dialog will catch those and ask the user to report them to the issues section in this repo.

[learn-more]

Fair enough, will make this somewhat more robust.

[activescott]

As with my other comment above, I don't like catch handlers with no exception listed. Please be explicit about the exception we expect here and let the other ones bubble up so the user reports them.

BTW: Specifically what I've seen happen is that you end up with some other weird behavior that you spend lots of time troubleshooting and and a catch like this (or even catch (Exception...)) is catching and hiding some exception that would have otherwise led you to the cause of an unrelated issue. Even more bizarre, is that C#/.NET will even catch unmanaged CRT exceptions with this type of catch handler without any exception listed, and that is super weird when that happens!

[learn-more]

Sorry I overlooked the "#137" in the PR description. I like the idea of portable, but now that I saw the PR it did make me think that in a "traditional install" for windows where apps are installed into "Program Files" apps won't be able to write into that directory. We don't actually provide an msi that installs there so it may not be common among lessmsi users. I also checked Chocolatey (which we do provide a popular package for) and apparently Chocolatey does allow writing to the app directory (I did not test though).

Since it is somewhat unorthodox to write data into the same directory as the app though, I wonder if maybe we shouldn't have a fallback to a user-specific, safe location to write settings if we don't have write/create permission or should we just fail and not save settings? What do you think?

One approach that comes to mind is this:
The most clean solution would probably be, first try to load it from the application folder,
Then fall back to %appdata%, and possibly even fall back to %localappdata%.

Since your requested a minimal change, I did not write code like that, but if you prefer that (and with a bit more robust error handling maybe) then I can rework the code into something like this.
Please note, that I might add another class depending on how much code there is going to be.

Do you think the location I chose for the new code 'Model' is correct?
If not, please suggest a better place.

[learn-more]

Left a couple minor notes on the code. If you could look at those then I'll do a deeper dive and test this one out and as long as I don't run into any obvious errors I'll merge.

Please do make sure that you have tested it. If you could please test in c:\Program Data... like chocolatey uses, and in c:\Program Files...\ (where it should fail gracefully) and confirm I will really appreciate it!

Thanks again for contributing!

I test all code submitted ;)
But I did not test it in multiple places, nor how it behaves in a readonly dir.
When we reach a consensus how to implement this exactly, I'll make sure to test the code in multiple places, and include in a new PR comment what I tested.

[activescott]

Sorry I overlooked the "#137" in the PR description. I like the idea of portable, but now that I saw the PR it did make me think that in a "traditional install" for windows where apps are installed into "Program Files" apps won't be able to write into that directory. We don't actually provide an msi that installs there so it may not be common among lessmsi users. I also checked Chocolatey (which we do provide a popular package for) and apparently Chocolatey does allow writing to the app directory (I did not test though).
Since it is somewhat unorthodox to write data into the same directory as the app though, I wonder if maybe we shouldn't have a fallback to a user-specific, safe location to write settings if we don't have write/create permission or should we just fail and not save settings? What do you think?

One approach that comes to mind is this:
The most clean solution would probably be, first try to load it from the application folder,
Then fall back to %appdata%, and possibly even fall back to %localappdata%.

Since your requested a minimal change, I did not write code like that, but if you prefer that (and with a bit more robust error handling maybe) then I can rework the code into something like this.
Please note, that I might add another class depending on how much code there is going to be.

Yeah, I don't think we need such robust fallback. I'm fine to just drop/loose the saved settings if it can't save for now. If people complain we can always add the feature to fallback to a user-specific directory later. Again I apologize for being a stickler, but I am guessing there are several thousand people using this utility regularly based on how many people download a new version in chocolatey in just a few days so I want to make sure it at least isn't crashing or particularly problematic for users.

Do you think the location I chose for the new code 'Model' is correct?
If not, please suggest a better place.

No concerns on the code location.

Thanks again and please to tag me when you want me to take another look.

[activescott]

@learn-more I'm also going to go ahead and close this since it's quite stale, but feel free to re-open it if you're interested. I'd be happy to pitch in and help you get it over the line!

[learn-more]

@activescott the net win for me is not big enough on this feature (I would not use it myself) to justify the risk of breaking something / the extra work required to properly validate it.

If someone wants to continue on this where I left off, they are free to take my code.
I am not sure how github handles PR's for deleted branches, but I will delete my branch for this somewhere in the future.