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

use package manager to get debug info instead of apm process #117

Merged
merged 1 commit into from
Apr 16, 2016

Conversation

andycmaj
Copy link

fix for #103.

should have same output, and doesn't seem to freeze atom when fetching large package lists.

@andycmaj
Copy link
Author

will get style issues fixed and update

@BinaryMuse
Copy link

/cc @lee-dohm

@lee-dohm
Copy link
Contributor

Active packages is not the same as installed packages in all cases. For example:

  1. When one or more packages are marked disabled
  2. When one or more packages are not yet activated because they have activation events defined that have not yet been satisfied
  3. When one or more packages are in the process of being updated

Have you tested to ensure that the output is the same even in the above cases?

@andycmaj
Copy link
Author

@lee-dohm so previously, looks like you got the equivalent of AvailablePackages from apm, which did include ALL packages, including disabled ones, and even ones that weren't ever loaded in the atom session notifications is reporting on.

My call to getActivePackages does only report active packages.

Active packages is not the same as installed packages in all cases

Do we really want to capture ALL of the installed packages? There were a few things that were not previously captured by that ALL package listing:

  • didn't indicate which packages were enabled/disabled (active/inactive).
  • didn't indicate which packages were actually Loaded. LoadedPackages can change if you disable a package then restart. Disabled packages are still loaded after you disable them but before restarting Atom

I can make the change to use getAvailablePackageMetadata instead of getActivePackages, but i'm not sure this is more helpful to devs trying to debug packages.
Thoughts?

@lee-dohm
Copy link
Contributor

  • The function is named getInstalledPackages. So at least the function name should change if we're not going to return all installed packages.
  • I would rather get the list of all packages (though indicating which are currently disabled would be nice) because many packages don't disable cleanly, so you can't be sure that just because it is disabled it isn't affecting things

@andycmaj
Copy link
Author

The function is named getInstalledPackages

Good point!

I would rather get the list of all packages (though indicating which are currently disabled would be nice).

Ok. Getting all packages should be somewhat straightforward. Comparing with Active package list might be a bit less performant. I'll do a bit of testing and see if this seems slow. If so, i'll just return ALL packages as apm did before.

@lee-dohm
Copy link
Contributor

Awesome, thanks so much for your work on this!

notifications debug info gets all installed packages and mark as inactive or active
@andycmaj
Copy link
Author

ok with my latest change, here's a sample of the output:

# User
advanced-open-file, v0.14.3 (inactive)
an-old-hope-syntax, v0.5.0 (inactive)
atom-monokai, v0.10.1 (inactive)
atom-yeoman, v0.3.15 (active)
ax-monokai-syntax, v0.1.0 (inactive)
base16-papercolor-dark-syntax, v0.2.0 (active)
base16-papercolor-light-syntax, v0.2.0 (inactive)
character-table, v0.4.3 (active)
chester-atom-syntax, v0.1.1 (inactive)
coffee-compile, v0.21.1 (active)
file-icons, v1.7.2 (active)
improved-chester-atom-syntax, v0.2.0 (inactive)
json-schema, v0.1.15 (inactive)
language-aspx, v0.4.0 (inactive)
language-babel, v2.17.2 (active)
language-cs, v0.1.1 (active)
language-cshtml, v0.1.1 (inactive)
language-docker, v1.1.6 (active)
lazy-motion, v0.3.0 (inactive)
linter, v1.11.4 (active)
linter-docker, v0.1.2 (active)
linter-eslint, v7.2.0 (active)
monokai-seti, v0.7.0 (inactive)
monokai-shade, v0.3.5 (inactive)
monokai-slate, v0.5.0 (inactive)
octocat-syntax, v0.1.5 (inactive)
omnisharp-atom, v0.28.0 (active)
pane-move-plus, v0.3.1 (inactive)
pretty-json, v1.0.3 (active)
relative-numbers, v0.5.1 (active)
rizzo-one, v0.5.0 (inactive)
robin-hood-syntax, v1.0.0 (inactive)
seti-syntax, v0.4.2 (inactive)
smalls, v0.2.0 (inactive)
source-preview, v0.5.0 (inactive)
source-preview-babel, v0.1.1 (active)
swap-selection, v0.3.0 (inactive)
vim-mode, v0.65.0 (inactive)
vim-mode-plus, v0.34.0 (active)
vim-surround, v0.8.1 (inactive)
atom-dark-syntax, v0.27.0 (inactive)
atom-dark-ui, v0.51.0 (inactive)
atom-light-syntax, v0.28.0 (inactive)
atom-light-ui, v0.43.0 (inactive)
base16-tomorrow-dark-theme, v1.1.0 (inactive)
base16-tomorrow-light-theme, v1.1.1 (inactive)
one-dark-ui, v1.2.0 (active)
one-light-ui, v1.2.0 (inactive)
one-dark-syntax, v1.2.0 (inactive)
one-light-syntax, v1.2.0 (inactive)
solarized-dark-syntax, v1.0.0 (inactive)
solarized-light-syntax, v1.0.0 (inactive)
about, v1.4.2 (active)
archive-view, v0.61.1 (active)
autocomplete-atom-api, v0.10.0 (active)
autocomplete-css, v0.11.0 (active)
autocomplete-html, v0.7.2 (active)
autocomplete-plus, v2.29.1 (active)
autocomplete-snippets, v1.10.0 (inactive)
autoflow, v0.27.0 (inactive)
autosave, v0.23.1 (active)
background-tips, v0.26.0 (active)
bookmarks, v0.38.2 (active)
bracket-matcher, v0.81.0 (active)
command-palette, v0.38.0 (inactive)
deprecation-cop, v0.54.1 (active)
dev-live-reload, v0.47.0 (active)
encoding-selector, v0.21.0 (active)
exception-reporting, v0.37.0 (inactive)
find-and-replace, v0.199.0 (inactive)
fuzzy-finder, v1.0.4 (active)
git-diff, v1.0.1 (active)
go-to-line, v0.30.0 (inactive)
grammar-selector, v0.48.1 (active)
image-view, v0.57.0 (active)
incompatible-packages, v0.25.1 (active)
keybinding-resolver, v0.35.0 (active)
line-ending-selector, v0.3.1 (active)
link, v0.31.1 (inactive)
markdown-preview, v0.158.0 (active)
metrics, v0.53.1 (inactive)
open-on-github, v1.0.1 (inactive)
package-generator, v1.0.0 (inactive)
settings-view, v0.235.1 (active)
snippets, v1.0.1 (active)
spell-check, v0.67.0 (active)
status-bar, v1.1.2 (active)
styleguide, v0.45.2 (inactive)
symbols-view, v0.112.0 (inactive)
tabs, v0.92.0 (active)
timecop, v0.33.1 (inactive)
tree-view, v0.205.0 (active)
update-package-dependencies, v0.10.0 (active)
welcome, v0.34.0 (active)
whitespace, v0.32.2 (active)
wrap-guide, v0.38.1 (active)
language-c, v0.51.1 (active)
language-clojure, v0.20.0 (active)
language-coffee-script, v0.46.1 (active)
language-csharp, v0.12.0 (active)
language-css, v0.36.0 (active)
language-gfm, v0.85.0 (active)
language-git, v0.12.1 (active)
language-go, v0.42.0 (active)
language-html, v0.44.1 (active)
language-hyperlink, v0.16.0 (active)
language-java, v0.17.0 (active)
language-javascript, v0.110.0 (active)
language-json, v0.17.6 (active)
language-less, v0.29.0 (active)
language-make, v0.21.0 (active)
language-mustache, v0.13.0 (active)
language-objective-c, v0.15.1 (active)
language-perl, v0.32.0 (active)
language-php, v0.37.0 (active)
language-property-list, v0.8.0 (active)
language-python, v0.43.0 (active)
language-ruby, v0.68.3 (active)
language-ruby-on-rails, v0.25.0 (active)
language-sass, v0.46.0 (active)
language-shellscript, v0.21.0 (active)
language-source, v0.9.0 (active)
language-sql, v0.20.0 (active)
language-text, v0.7.1 (active)
language-todo, v0.27.0 (active)
language-toml, v0.18.0 (active)
language-xml, v0.34.4 (active)
language-yaml, v0.25.1 (active)

# Dev
notifications, v0.63.1 (active)

@lee-dohm
Copy link
Contributor

And inactive here can mean either disabled or not yet activated for one reason or another?

@lee-dohm
Copy link
Contributor

This looks really good. I'm going to sleep on it and go over it with fresh 👀 tomorrow. It should be good to go. Thanks again for all the work!

@lee-dohm lee-dohm merged commit 4891702 into atom:master Apr 16, 2016
@lee-dohm
Copy link
Contributor

Thanks for contributing!

@andycmaj
Copy link
Author

andycmaj commented Apr 16, 2016

Thanks, yeah... it probably could be more efficient (i'm still getting used to coffee idioms), but it's still way faster than apm dumps.

And inactive here can mean either disabled or not yet activated for one reason or another?

yeah. exactly. like you mentioned... packages that are conditionally loaded will be listed as inactive if the condition hasn't been triggered yet.

please let me know if it needs any more work once you have had a chance to dogfood a bit.

also, not sure what the release process is, but #103 should probably be closed at some point.

@andycmaj
Copy link
Author

actually, thinking more about it, this probably can be optimized a bit by making a few small changes to the PackageManager apis. For example, the fact that atom.packages.getAvailablePackageMetadata() results don't include package paths or whether or not the package is dev, so you need to make a separate call to atom.packages.getAvailablePackagePaths() and check whether the package is in the /dev/ path, and then later have to do an O(n^2) lookup for each available package to check if it's in the devPackageNames array...

just little stuff. Lemme know if you think it's worth optimizing by making more changes to package manager.

@lee-dohm
Copy link
Contributor

also, not sure what the release process is, but #103 should probably be closed at some point.

Typically you put "Fixes #XXX" in the body of the PR comment so that when it gets merged, the Issue gets automatically closed. Thanks for reminding me to close it 😀

this probably can be optimized a bit

Probably, but it was noticeably faster than what we had. There are a few different ways to optimize the resulting code. At this point, I'd rather not change the API for atom.packages for just this one use case. When I get some free time, I may come back to this to fiddle with it ... but like I say, it is much better than before.

@andycmaj
Copy link
Author

👍

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

Successfully merging this pull request may close these issues.

4 participants