Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

added new settings dialog + settings manager#113

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ibetitsmike merged 18 commits intomainfrommike/57-starting-coder-desktop-on-boot
Jun 10, 2025

Conversation

ibetitsmike
Copy link
Contributor

@ibetitsmikeibetitsmike commentedMay 29, 2025•
edited
Loading

Closes:#57 &#55

Adds:

  • SettingsManager that manages settings located in AppData
  • Settings views to manage the settings
  • StartupManager that allows to control registry access to enable load on startup

image

matifali reacted with heart emoji
@ibetitsmikeibetitsmike marked this pull request as ready for reviewJune 2, 2025 11:14
@ibetitsmikeibetitsmikeforce-pushed themike/57-starting-coder-desktop-on-boot branch from9037758 tobad5320CompareJune 3, 2025 13:54
string exe = Process.GetCurrentProcess().MainModule!.FileName;
try
{
using var key = Registry.CurrentUser.OpenSubKey(RunKey, writable: true)
Copy link
Member

Choose a reason for hiding this comment

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

Is there any (easy) way we can clean this up on uninstall?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

We can, but this will also basically clear the setting every team you update (I believe you run the uninstall then too, right)?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Follow up here:#121

/// </summary>
bool ConnectOnLaunch { get; set; }
/// <returns></returns>
public T? GetFromCache();
Copy link
Member

Choose a reason for hiding this comment

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

Rather than have this,Read should probably return from cache by default. I know this goes against a point I made in a previous review but if we're going with the Read/Write API then it should just be all in

Copy link
Member

Choose a reason for hiding this comment

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

Also, Read should probably not fail if it failed to read/parse the file. IDK what good behavior is, but I think for now just returning a default config is OK

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

This is definitely going against this comment :D
#113 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I'm like 50/50 on whether we should crash or just use the default config... IDK what the best option is. For now with a single option maybe it's fine to just keep going, but in the future if we add config options that are more sensitive to being clobbered it might be bad to just ignore a parse failure...

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

it throws an exception now and I think that's OK considering - if you fail to operate the file there's a high likelihood that the feature won't work ata ll.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's fair. We can revisit this if it's causing issues

@ibetitsmikeibetitsmike merged commit059179c intomainJun 10, 2025
3 checks passed
@ibetitsmikeibetitsmike deleted the mike/57-starting-coder-desktop-on-boot branchJune 10, 2025 13:56
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@deansheatherdeansheatherdeansheather approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support starting Coder Desktop on boot
2 participants
@ibetitsmike@deansheather
[8]ページ先頭
©2009-2025 Movatter.jp