-
Notifications
You must be signed in to change notification settings - Fork 17.3k
Conversation
I think there are going to be a few failing tests that we'll need to figure out. That's what I'm seeing locally. Overall, it doesn't look too bad. |
I realized later that this is still crashing when run in standard mode (non-dev). |
I tried disabling snapshot generation in the build script, and Atom starts up without crashing in non-dev mode. |
I've been narrowing down the cause of the crashes, and I've gotten down to this diff: diff --git a/src/config.coffee b/src/config.coffee
index b8bf8a7..67b00d3 100644
--- a/src/config.coffee
+++ b/src/config.coffee
@@ -1064,7 +1064,7 @@ class Config
continue unless scopeSchema.hasOwnProperty('default')
scopedDefaults[scope] = {}
setValueAtKeyPath(scopedDefaults[scope], keyPath, scopeSchema.default)
- @scopedSettingsStore.addProperties('schema-default', scopedDefaults)
+ #@scopedSettingsStore.addProperties('schema-default', scopedDefaults)
if schema.type is 'object' and schema.properties? and isPlainObject(schema.properties)
keys = splitKeyPath(keyPath)
@@ -1140,7 +1140,7 @@ class Config
settingsBySelector = {}
settingsBySelector[selector] = value
- @scopedSettingsStore.addProperties(source, settingsBySelector, priority: @priorityForSource(source))
+ #@scopedSettingsStore.addProperties(source, settingsBySelector, priority: @priorityForSource(source))
@emitChangeEvent()
getRawScopedValue: (scopeDescriptor, keyPath, options) ->
diff --git a/src/package-manager.js b/src/package-manager.js
index 17a5f22..a4d5a8a 100644
--- a/src/package-manager.js
+++ b/src/package-manager.js
@@ -511,7 +511,7 @@ module.exports = class PackageManager {
pack = metadata.theme ? new ThemePackage(options) : new Package(options)
pack.preload()
- this.preloadedPackages[packageName] = pack
+ //this.preloadedPackages[packageName] = pack
return pack
} With these lines commented out, Atom no longer crashes. |
Lately, I've been modifying the startup script "out/startup.js" directly, creating a new snapshot, and starting up Atom to debug the crashes. To create a new snapshot:
The feedback loop is a bit faster. |
Note: commenting out the package preload stops the crashes i.e. //pack.preload()
//this.preloadedPackages[packageName] = pack |
Yet another twist: In the atom repo with a snapshot blob from a prior run of
You should see Atom load without crashing. |
I've narrowed things down a bit further. If I made the following changes in "./out/startup.js" and create a new snapshot, Atom does not crash: Comment out this line:
Change the parse function in atom-slick to:
Then, I create a new snapshot and run Atom:
|
I found another patch that doesn't crash. Besides all the language packages (no idea why those are causing crashes), it only removes two packages (github and whitespace). diff --git a/package.json b/package.json
index cca8ea9..e1df733 100644
--- a/package.json
+++ b/package.json
@@ -109,7 +109,6 @@
"exception-reporting": "0.42.0",
"find-and-replace": "0.215.0",
"fuzzy-finder": "1.7.3",
- "github": "0.8.3",
"git-diff": "1.3.6",
"go-to-line": "0.32.1",
"grammar-selector": "0.49.8",
@@ -134,41 +133,13 @@
"tree-view": "0.221.3",
"update-package-dependencies": "0.13.0",
"welcome": "0.36.6",
- "whitespace": "0.37.5",
"wrap-guide": "0.40.3",
- "language-c": "0.58.1",
"language-clojure": "0.22.5",
- "language-coffee-script": "0.49.3",
- "language-csharp": "0.14.3",
- "language-css": "0.42.8",
- "language-gfm": "0.90.2",
- "language-git": "0.19.1",
- "language-go": "0.44.3",
- "language-html": "0.48.3",
"language-hyperlink": "0.16.3",
- "language-java": "0.27.6",
- "language-javascript": "0.127.7",
- "language-json": "0.19.1",
- "language-less": "0.34.1",
- "language-make": "0.22.3",
"language-mustache": "0.14.4",
- "language-objective-c": "0.15.1",
- "language-perl": "0.38.1",
- "language-php": "0.42.2",
- "language-property-list": "0.9.1",
- "language-python": "0.45.5",
- "language-ruby": "0.71.4",
"language-ruby-on-rails": "0.25.2",
- "language-sass": "0.61.3",
- "language-shellscript": "0.25.4",
- "language-source": "0.9.0",
- "language-sql": "0.25.8",
"language-text": "0.7.3",
- "language-todo": "0.29.3",
- "language-toml": "0.18.1",
- "language-typescript": "0.2.3",
- "language-xml": "0.35.2",
- "language-yaml": "0.31.1"
+ "language-todo": "0.29.3"
},
"private": true,
"scripts": {
diff --git a/src/initialize-application-window.coffee b/src/initialize-application-window.coffee
index f8f670c..ec5469b 100644
--- a/src/initialize-application-window.coffee
+++ b/src/initialize-application-window.coffee
@@ -29,7 +29,7 @@ if global.isGeneratingSnapshot
require('dalek')
require('find-and-replace')
require('fuzzy-finder')
- require('github')
+ #require('github')
require('git-diff')
require('go-to-line')
require('grammar-selector')
@@ -54,7 +54,7 @@ if global.isGeneratingSnapshot
require('tree-view')
require('update-package-dependencies')
require('welcome')
- require('whitespace')
+ #require('whitespace')
require('wrap-guide')
clipboard = new Clipboard |
All the whitelisted languages don't contain a settings folder, except for language-clojure. |
electron 1.7.10 has been released which seems to fix a problem with nvidia graphic cards on mac os high sierra. Maybe we could update to 1.7.10 directly instead of 1.7.9 |
This won't work until v8 6.2.
This commit uses `content` containment (i.e. `layout paint style`) as opposed to `strict` containment (i.e. `layout paint style size`) for dummy scrollbar elements. By removing `size` containment we are fixing a rendering bug that was preventing the scrollbar from being sized correctly. This problem was caught by a TextEditorComponent test (https://circleci.com/gh/atom/atom/6393).
Okay, I think this might be ready for a test drive. I have fixed the failing tests and, along with @nathansobo and @leroix, we finally figured out why snapshots were making Atom crash during startup (see electron/electron#11387 for all the details). The good news there is that it was a bug in V8 which is already fixed on V8 6.3 and we just have to wait until a version of Electron that embeds such version gets released (presumably Electron 1.9 or later). @ungb @rsese @Ben3eeE @50Wliu: can you help us test this pull-request? After a cursory look everything seems reasonable. I briefly tested the IME interaction and it seemed to work correctly, although I'd love to get a more thorough QA pass on that, especially since I have removed some workarounds that we added specifically for Chrome 56 in 69799d3. |
For the following test, it seems that the Key binding with composition characters are ignored #15189macOS: This should be tested on macOS Steps to repro:
# general
'atom-text-editor':
# use jkl; vim-like when pressing alt (word by word, paragraph by paragraph)
# or alt (letter by letter)
'alt-h': 'core:move-left'
'alt-j': 'core:move-down'
'alt-k': 'core:move-up'
'alt-l': 'core:move-right'
Expected: the keybindings work Issues we've seen: it types out the modified keys and ignores the key mapping I've tried stable and it seems to be the same behavior so this isn't a regression I suppose. |
☝️
Fixes #15696
Fixes #15737