Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Viewing a package's settings should activate it #356

Closed
lee-dohm opened this issue Jan 27, 2015 · 22 comments · Fixed by #371
Closed

Viewing a package's settings should activate it #356

lee-dohm opened this issue Jan 27, 2015 · 22 comments · Fixed by #371

Comments

@lee-dohm
Copy link
Contributor

See this post on Discuss for full details.

Repro Steps

  1. Create a package that has activationCommands set
  2. Add a config schema to the package
  3. Launch Atom
  4. Open Settings View with Cmd+,
  5. Navigate to the package's settings

Expected: To see the configured default settings
Actual: The package's settings are blank

@lee-dohm lee-dohm added the bug label Jan 27, 2015
@50Wliu
Copy link
Contributor

50Wliu commented Jan 27, 2015

👍, ran into this when trying to create my own package. Was really confused as to why I couldn't get settings to show up inside the Settings View.

@braver
Copy link
Contributor

braver commented Jan 28, 2015

Possibly related to #347?

@thedaniel
Copy link
Contributor

Given that packages can and do change the editor state significantly on activation, we probably can't just activate them when their settings are viewed.

@dschwen
Copy link

dschwen commented Feb 2, 2015

I don't see why atom couldn't just require the main package file, inspect the config member, but not run activate. Do we really have packages where the mere loading without actual initialization through activate takes a long time?

@moxley
Copy link

moxley commented Feb 3, 2015

@dschwen: I agree that it seems odd that the config property cannot be inspected without also calling activate. However, to your point about taking a long time to initialize, I commented out the body of activate in my package so that it does nothing, then reloaded Atom, and Atom reported that it took 250ms to load my package. That is compared to 25ms it took to load the ascii-art tutorial package. Maybe the main class isn't the ideal place to put the config. It's declarative code, and it's a standard object recognized by Atom, so it follows that it should live in a json or cson file, like other such declarative code.

@dschwen
Copy link

dschwen commented Feb 3, 2015

Sure, a separate cson file would be great. I was just hoping some less invasive change could be made, that didn't require updating all packages. What about caching the config defaults? And lazily check if the cache needs to be updated using a web worker after startup?

@Arcath
Copy link
Contributor

Arcath commented Feb 10, 2015

@thedaniel My pull request uses Package.activateConfig() which doesn't activate the whole package saving settings-view from being the cause of random behaviour as packages get activated left right an centre.

@thedaniel
Copy link
Contributor

I left a comment re: @Arcath's use of activateConfig() over on #371

@mostafahussein
Copy link

When will this issue be solved ? should we wait until atom 0.180.0 ?

@thedaniel
Copy link
Contributor

@Code-Vortex There isn't a target version or release date for closing this issue.

@mostafahussein
Copy link

is there any work around that we can use until it got fixed ?

@Arcath
Copy link
Contributor

Arcath commented Feb 23, 2015

Activate the package then open settings. So just run anything in the
command pallette for the required package
On 23 Feb 2015 09:15, "Mostafa Hussein" notifications@github.com wrote:

is there any work around that we can use until it got fixed ?


Reply to this email directly or view it on GitHub
#356 (comment).

@mostafahussein
Copy link

@Arcath well , this will makes you able to use commands which already provided by some package, but you wont be able to select anything from a drop down list or check on something.

@Glavin001
Copy link
Contributor

This is an important bug, and it is very shocking to users (see above for links to here from other projects) when they open up their package's settings and see next to nothing. Since discussions are still ongoing for over a month, might I recommend that you (maintainers of Atom) accept the loading time presented by the temporary workaround of loading the package when the Settings view shows the package. For now, it will have a slow-ish loading time -- 250ms is not great when you're loading 10+ packages, but when viewing a single one, does it really bother anyone that much? -- however then we at least be able to actually see the Package's settings. Then once you can agree on a proper solution, change over to that and upgrade the User's experience. Right now the experience is discomforting: no settings, slow or not, at all. I think actually being able to see the Package's settings is more valuable and important to me than waiting even a few seconds, don't you?

@moxley
Copy link

moxley commented Mar 8, 2015

@Glavin001 Thank you for highlighting the importance of this bug. I agree 100%.

@lee-dohm: perhaps we can retitle this bug to reflect the problem only (not the potential solution), to make the severity more apparent. Including a solution in a bug report can have a negative impact on the efficiency of resolving the bug. A potential title would be "Unable to see or modify package settings".

@thomasjo
Copy link
Contributor

thomasjo commented Mar 8, 2015

I simply worked around this by not relying on activationCommands anymore. Puts more responsibility on package developers if they still want delayed activation in order to not negatively impact startup time, but entirely doable.

I'd of course prefer to see this issue resolved, but let's not blow this out of proportion since it's relatively straightforward to work around the problem. I'd be happy to piece together an example package that handles it's own delayed activation. Just let me know.

ChanceM added a commit to ChanceM/atom-hashrocket that referenced this issue Mar 13, 2015
Removed activation events so settings will populate properly. See this
issue: atom/settings-view#356
prrrnd added a commit to prrrnd/atom-git-projects that referenced this issue Mar 18, 2015
@DavidLGoldberg
Copy link

Just wanted to weigh in. I love atom and all of the hard work going into it. But I do find this to be a very important / frustrating bug. Not just for users, but as a package developer. I spent some time reading and re reading activation documentation thinking I was doing this wrong. "It can't be that bad of a bug", I kept thinking.

I finally found the post in discuss pointing to this.

I think with current load issues we probably really do need to be using activation events (for the most part). I like the fact the config values are in coffee and therefore could be created dynamically (I've never done that but interesting). This can still be achieved by pulling this out into it's own coffee file for clarity and this would make it pretty easy to wire up that into atom start up and not be tied into the rest of the package activation. It would also probably be easier conceptually.

So in conclusion, I just wanted to weigh in that I find it as surprising to the package developer as it is for the regular user. Less work / barriers for package developers makes for a better Atom community.

Edit: To add, I forgot to mention. For some reason when I opened the settings I saw 2/4 settings and they did not have their descriptions set. Upon activation I see all 4 with descriptions. Very, surprising behavior.

@JoshCheek
Copy link

I don't understand why the config isn't just moved to package.json It should be able to portray all the relevant values via the JSON schema. I've had difficulty getting it to do that, but that's a separate issue.

While that makes the most sense to my brain, it's possible that the issue deals with customized default values that can't be known without running code. If this is the case, then a post-install callback might be in order. It coudl be used to set the initial values into the settings.

Or, maybe require the main file when the user accesses the settings page, but don't activate it. Then the cost of requiring the file is deferred until it is known to be necessary, but the package is not activated and so does not go meddling with the environment.

As of right now, my users are still editing confg.cson by hand.

@thedaniel
Copy link
Contributor

I don't understand why the config isn't just moved to package.json

Because it's possible to create config dynamically in the public 1.0 API so we can't break it. I agree it's better to do it statically and this is likely to be the requirement in Atom 2.0. I think we'll probably add something for static config and strongly encourage it before then, even if we can't strictly enforce it.

@izuzak
Copy link
Contributor

izuzak commented Apr 8, 2015

Reported again over at: atom/toggle-quotes#19

@hedefalk
Copy link

I struck this developing a package and want to weigh in. I need to have a setting that the user need to set before activating the package. To be concrete, the package needs to run a command "sbt" that the users need to point out. I've defaulted it to '/usr/local/bin/sbt' but that's just my system. That's for bootstrapping the package so it's kind of catch 22 not to be able to set it before using it.

@mostafahussein
Copy link

by updating the package manually using apm install settings-view issue will be solved (manually method in case you don't want to wait until you receive an auto update for it) resolved in #371, so this closes travs/markdown-pdf#42, closes #383, closes moxley/atom-ruby-test#5, closes #383, closes Arcath/jekyll-atom#9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.