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

feat: Store settings in the application folder #160

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/LessMsi.Gui/LessMsi.Gui.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
<SubType>Form</SubType>
</Compile>
<Compile Include="MainFormPresenter.cs" />
<Compile Include="Model\ApplicationSettings.cs" />
<Compile Include="Model\MsiFileItemView.cs" />
<Compile Include="Model\MsiPropertyInfo.cs" />
<Compile Include="Model\CabContainedFileView.cs" />
Expand Down
73 changes: 73 additions & 0 deletions src/LessMsi.Gui/Model/ApplicationSettings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
using System.Collections.Generic;
using System.IO;
using System.Reflection;
using System.Xml.Serialization;

namespace LessMsi.Gui.Model
{
public class ApplicationSettings
{
static string applicationSettingsFile;
static ApplicationSettings applicationSettings;

public List<string> RecentFiles { get; set; } = new List<string>();


public void Save()
{
try
{
using (TextWriter writer = new StreamWriter(ApplicationSettingsFile))
{
XmlSerializer serializer = new XmlSerializer(typeof(ApplicationSettings));
serializer.Serialize(writer, this);
}
}
catch
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, will make this somewhat more robust.

{
// Should we nag the user?
}
}

static string ApplicationSettingsFile
{
get
{
if (applicationSettingsFile == null)
{
string directory = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
applicationSettingsFile = Path.Combine(directory, "lessmsi-gui-settings.xml");
}
return applicationSettingsFile;
}
}

public static ApplicationSettings Default
{
get
{
if (applicationSettings == null)
{
try
{
using (FileStream fs = new FileStream(ApplicationSettingsFile, FileMode.Open))
{
XmlSerializer serializer = new XmlSerializer(typeof(ApplicationSettings));
applicationSettings = (ApplicationSettings)serializer.Deserialize(fs);
}
}
catch
Copy link
Owner

Choose a reason for hiding this comment

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

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!

{
applicationSettings = null;
}
if (applicationSettings == null)
{
applicationSettings = new ApplicationSettings();
}
}

return applicationSettings;
}
}
}
}
9 changes: 5 additions & 4 deletions src/LessMsi.Gui/MruMenuStripManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
// Authors:
// Scott Willeke ([email protected])
//
using LessMsi.Gui.Model;
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
Expand Down Expand Up @@ -66,7 +67,7 @@ public MruMenuStripManager(ToolStripMenuItem placeHolderItem)

private void LoadPreferences()
{
var recentFiles = Properties.Settings.Default.RecentFiles;
var recentFiles = ApplicationSettings.Default.RecentFiles;
if (recentFiles == null)
return;
var paths = new string[recentFiles.Count];
Expand All @@ -85,9 +86,9 @@ private void LoadPreferences()
public void SavePreferences()
{
PruneItems();
var paths = new StringCollection();
_items.ForEach((item) => paths.Add(item.FilePathName));
Properties.Settings.Default.RecentFiles = paths;
ApplicationSettings.Default.RecentFiles.Clear();
_items.ForEach((item) => ApplicationSettings.Default.RecentFiles.Add(item.FilePathName));
activescott marked this conversation as resolved.
Show resolved Hide resolved
ApplicationSettings.Default.Save();
}

/// <summary>
Expand Down
15 changes: 2 additions & 13 deletions src/LessMsi.Gui/Properties/Settings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions src/LessMsi.Gui/Properties/Settings.settings
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
<?xml version='1.0' encoding='utf-8'?>
<SettingsFile xmlns="http://schemas.microsoft.com/VisualStudio/2004/01/settings" CurrentProfile="(Default)" GeneratedClassNamespace="LessMsi.Gui.Properties" GeneratedClassName="Settings">
<SettingsFile xmlns="http://schemas.microsoft.com/VisualStudio/2004/01/settings" CurrentProfile="(Default)">
<Profiles />
<Settings>
<Setting Name="RecentFiles" Type="System.Collections.Specialized.StringCollection" Scope="User">
<Value Profile="(Default)" />
</Setting>
</Settings>
<Settings />
</SettingsFile>