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

Prompts for restart when certain core settings aren't default #19323

Closed
Arcanemagus opened this issue May 13, 2019 · 10 comments · Fixed by #19354

Comments

@Arcanemagus
Copy link
Member

commented May 13, 2019

Prerequisites

Description

If one of the core setting that are configured to prompt for a restart are changed from their default value then Atom will prompt for a restart every time it starts up.

Steps to Reproduce

  1. Change a core setting from the following list to a non-default value:
    • core.titleBar
    • core.colorProfile
    • core.fileSystemWatcher
  2. Restart Atom

Expected behavior: [What you expect to happen]
No prompt to restart Atom on launch.

Actual behavior: [What actually happens]
Prompts to restart Atom on every launch, regardless of how many times you restart it.

Reproduces how often: [What percentage of the time does it reproduce?]
100% of the time.

Versions

OS: Windows 10 x64

Atom    : 1.39.0-nightly2
Electron: 2.0.18
Chrome  : 61.0.3163.100
Node    : 8.9.3
apm  2.1.7
npm  6.2.0
node 8.9.3 x64
atom 1.39.0-nightly2
python 2.7.15
git 2.19.1.windows.1
visual studio 2015

Additional Information

N/A

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

@Arcanemagus hey, do you think you could give this another try once we get a dev build that includes #19325?

If you're on Windows, once this AppVeyor build goes green (modulo any flakiness), you should be able to pull down an artifact from the dev channel. I couldn't reproduce your issues once that fix was in, and I couldn't even start Atom before #19321 was merged, so I'm curious where the latest fixes leave things for you.

@Arcanemagus

This comment has been minimized.

Copy link
Member Author

commented May 13, 2019

I'm still seeing the same thing on that build. Is there any additional information I can provide?

@asheren asheren referenced this issue May 14, 2019
4 of 9 tasks complete
@nathansobo

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

Any better after the latest changes on master?

@Arcanemagus

This comment has been minimized.

Copy link
Member Author

commented May 15, 2019

Tested with the atom-x64-windows.zip build from here (216404d) and I'm still seeing this.

@50Wliu

This comment has been minimized.

Copy link
Member

commented May 16, 2019

Can confirm what @Arcanemagus is seeing.

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

We're not going to take this path and I have changed the default back to nsfw.

@nathansobo nathansobo closed this May 16, 2019

@Arcanemagus

This comment has been minimized.

Copy link
Member Author

commented May 16, 2019

Although the default has been changed back to native, this still happens on builds after #19345 was merged.

@Arcanemagus Arcanemagus reopened this May 16, 2019

@Arcanemagus

This comment has been minimized.

Copy link
Member Author

commented May 16, 2019

I added some logging to the onDidChange binding and it looks like the value is being initialized to the default value (native) and then changed to the user's custom value (atom in my case), which is triggering the onDidChange event.

Edit: Confirmed that if I change the setting to native (the current default) I don't see this. Tested on a build based on b79d908.

@Arcanemagus Arcanemagus changed the title Prompts for restart when @atom/notify watcher not selected Prompts for restart when certain core settings aren't default May 16, 2019

@Arcanemagus

This comment has been minimized.

Copy link
Member Author

commented May 16, 2019

Updated the description: This actually is only tangentially related to the file system watcher changes.

It turns out that any of these settings will always prompt for a restart if they are changed from the default value.

We likely need to put this event subscription to somewhere in the startup process after user settings have first been applied.

@nathansobo

This comment has been minimized.

Copy link
Contributor

commented May 17, 2019

I think this was introduced in #19246. On Windows, we're no longer awaiting loading of the config file for performance reasons since it's really slow on Electron 2. I have a fix in the works, but I need to revert the @atom/notify stuff before building on top of it. I'll need to cherry-pick my fix to beta later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.