Atom Shell #1756

Merged
merged 106 commits into from Jul 2, 2015

Conversation

Projects
None yet
@MikeInnes
Contributor

MikeInnes commented Nov 30, 2014

Figured I'd turn Chris's work on #1728 into a PR so we can all discuss what needs doing and make changes. Feel free to edit this with new tasks.

  • Sort out inconsistencies with quitting (Atom, at least on OS X, distinguishes between closing windows and quitting).
  • Porting CLI
    • drag/drop opening
    • open files
    • open from file manager during and after startup
  • Save and restore window position
  • Previous workspace isn't loading on startup
  • Windows Testing
  • Linux Testing
  • Packaging Build Step
  • Port nw package.json options to atom-shell. References: nw options and browser-window options
    • Figure out why icon option doesn't work
  • Sort out JS loading (JQuery isn't working for me any more)
  • Keybinding issues
    • Esc doesn't work in command bar
  • Resolve error loading node modules when pwd is not the app directory
  • File save dialogs are broken
  • Project search leaves working indicator on (ok, my workspace is just really huge)
  • Julia plugin
    • Interactive plots broken
  • platform/open no worky
  • Binary and app updates
  • script/build.sh (to replace *_deps.sh)
    • build on osx and functions correctly
    • clone plugins
    • copy over executables e.g. light
    • handle plugins/node/node{,.exe}
    • rename app - may need to workaround this issue on osx
    • git add and use relevant contents of zip files
      • Mac: port plist
    • osx issues: console printing is wonky, icon doesn't show up, initial focus is inconsistent
    • build on linux - done on 64bit ubuntu 14.04
    • build on windows - done on 32 bit windows 7
  • mac menu issues: Select All and Cut don't work. This issue also exists with node-webkit and only happens when vim is enabled.
  • Port proxy support (we don't have a way to test this)
  • Evaling into browser tabs
  • Windows issues
    • cursor issues (doesn't change when resizing tabs) - not observed in windows 7
    • closing last window doesn't close app
    • opening file from cygwin terminal doesn't work
  • Linux issues
    • opening files from commandline doesn't work
    • worker syntax error - not observed in ubuntu 14
  • Updates to LT should be driven by main.js so that they can be guaranteed to only happen once

Install Instructions:
See doc/developer-install.md for full instructions

For OSX:

$ script/build.sh
# open from commandline
$ builds/lighttable-0.8.0-mac/light
# OR open executable
$ open -a $PWD/builds/lighttable-0.8.0-mac/LightTable.app

For Linux:

$ script/build.sh
# open from commandline
$ builds/lighttable-0.8.0-linux/light
# OR open executable
$ open -a $PWD/builds/lighttable-0.8.0-linux/LightTable

For Windows:

$ script/build.sh
# open executable
$ builds/lighttable-0.8.0-windows/LightTable.exe
@ibdknox

This comment has been minimized.

Show comment
Hide comment
@ibdknox

ibdknox Nov 30, 2014

Member

added more tasks in there.

Member

ibdknox commented Nov 30, 2014

added more tasks in there.

@cldwalker

This comment has been minimized.

Show comment
Hide comment
@cldwalker

cldwalker Dec 2, 2014

Member

Ported most of the nw package.json options. Doc links in above task.
@ibdknox I didn't see equivalents for js-flags, some of the chromium-args, no-edit-menu or toolbar but perhaps you may recognize their equivalents

Member

cldwalker commented Dec 2, 2014

Ported most of the nw package.json options. Doc links in above task.
@ibdknox I didn't see equivalents for js-flags, some of the chromium-args, no-edit-menu or toolbar but perhaps you may recognize their equivalents

@ibdknox

This comment has been minimized.

Show comment
Hide comment
@ibdknox

ibdknox Dec 2, 2014

Member

@cldwalker I think there's an API for reading the package.json that is provided by the atom-shell. The missing things shouldn't matter anymore except for maybe one or two of the chromium-args. Threaded-compositing used to cause all sorts of issues.

Member

ibdknox commented Dec 2, 2014

@cldwalker I think there's an API for reading the package.json that is provided by the atom-shell. The missing things shouldn't matter anymore except for maybe one or two of the chromium-args. Threaded-compositing used to cause all sorts of issues.

@MikeInnes

This comment has been minimized.

Show comment
Hide comment
@MikeInnes

MikeInnes Dec 8, 2014

Contributor

Interestingly, atom-shell seems to change the results of fuzzy searching. I used to be able to type rwi to reset the working indicator.

@ibdknox Any ideas why this might be? It's not an issue as-is, but just in case it points to something deeper

Contributor

MikeInnes commented Dec 8, 2014

Interestingly, atom-shell seems to change the results of fuzzy searching. I used to be able to type rwi to reset the working indicator.

@ibdknox Any ideas why this might be? It's not an issue as-is, but just in case it points to something deeper

@MikeInnes

This comment has been minimized.

Show comment
Hide comment
@MikeInnes

MikeInnes Dec 8, 2014

Contributor

With the relative path fixes you can now run ./deploy/run.sh &, which is a bit more convenient (for me at least).

Contributor

MikeInnes commented Dec 8, 2014

With the relative path fixes you can now run ./deploy/run.sh &, which is a bit more convenient (for me at least).

@MikeInnes MikeInnes referenced this pull request in JuliaIDE/Julia-LT Dec 10, 2014

Closed

Create a self-sufficient distribution #115

@MikeInnes

This comment has been minimized.

Show comment
Hide comment
@MikeInnes

MikeInnes Dec 10, 2014

Contributor

FYI I just rebased this to get the LT updates, if you're working on / using this branch you'll want to reset it.

Contributor

MikeInnes commented Dec 10, 2014

FYI I just rebased this to get the LT updates, if you're working on / using this branch you'll want to reset it.

MikeInnes added some commits Dec 10, 2014

fix windows issue
christ, get it together, windows
@cldwalker

This comment has been minimized.

Show comment
Hide comment
@cldwalker

cldwalker Dec 16, 2014

Member

Taking a look at cli tonight or tomorrow

Member

cldwalker commented Dec 16, 2014

Taking a look at cli tonight or tomorrow

cldwalker added some commits Dec 16, 2014

Open files from the commandline
Opening a file required a callback to know when window is loaded. Also removed unused app/extract!
and added docstrings for ipc.
Revert "ignore node_modules" which clobbers files in deploy/core/node…
…_modules.

@one-more-minute If this is still used, a more specific gitignore is needed.
This reverts commit b3535a1.

ibdknox added some commits Feb 9, 2015

move to latest atom-shell
Signed-off-by: Chris Granger <ibdknox@gmail.com>
upgrade to the latest clojurescript and fix a couple warnings/build e…
…rrors

Signed-off-by: Chris Granger <ibdknox@gmail.com>
@cldwalker

This comment has been minimized.

Show comment
Hide comment
@cldwalker

cldwalker Mar 6, 2015

Member

It's great to see we're upgrading cljs. I'm noticing some quirks around the upgrade:

  • Escaping to normal mode doesn't work in vim
  • Eval doesn't work (for html and cljs at least)
  • A couple of my plugins don't work and I suspect compiled cljs compatibility issues. I tried building with the "build file or project" command and saving a .cljs file but the recompile had no effect.

I'll look into this last issue some more.
I also noticed cljsDeps.js was recompiled. How do we compile that for future cljs upgrades?

Member

cldwalker commented on 8e73f59 Mar 6, 2015

It's great to see we're upgrading cljs. I'm noticing some quirks around the upgrade:

  • Escaping to normal mode doesn't work in vim
  • Eval doesn't work (for html and cljs at least)
  • A couple of my plugins don't work and I suspect compiled cljs compatibility issues. I tried building with the "build file or project" command and saving a .cljs file but the recompile had no effect.

I'll look into this last issue some more.
I also noticed cljsDeps.js was recompiled. How do we compile that for future cljs upgrades?

This comment has been minimized.

Show comment
Hide comment
@ibdknox

ibdknox Mar 6, 2015

Member

Yeah I noticed the weirdness too. I'm going to upgrade the clojure plugin's cljs and see if that fixes it. My thought is that if we're already making a fairly scary change that's going to break compatibility we should probably do all of them at once. :( Better to rip the bandaid off and fix everything in one go as opposed to constantly requiring people to update their plugins. I'll start a thread about it to see if that's consensus.

I also noticed cljsDeps.js was recompiled. How do we compile that for future cljs upgrades?

I just compile this:

(ns cljsdeps.core
  (:require [clojure.string :as string]
            [clojure.walk :as walk]
            [cljs.reader :as reader]))

With optimizations :simple and take the resulting output.

Member

ibdknox replied Mar 6, 2015

Yeah I noticed the weirdness too. I'm going to upgrade the clojure plugin's cljs and see if that fixes it. My thought is that if we're already making a fairly scary change that's going to break compatibility we should probably do all of them at once. :( Better to rip the bandaid off and fix everything in one go as opposed to constantly requiring people to update their plugins. I'll start a thread about it to see if that's consensus.

I also noticed cljsDeps.js was recompiled. How do we compile that for future cljs upgrades?

I just compile this:

(ns cljsdeps.core
  (:require [clojure.string :as string]
            [clojure.walk :as walk]
            [cljs.reader :as reader]))

With optimizations :simple and take the resulting output.

This comment has been minimized.

Show comment
Hide comment
@ibdknox

ibdknox Mar 8, 2015

Member

I rolled this back as it looks like it'll require more investigation into why things aren't working. Just recompiling plugins with the latest CLJS didn't seem to fix it.

Member

ibdknox replied Mar 8, 2015

I rolled this back as it looks like it'll require more investigation into why things aren't working. Just recompiling plugins with the latest CLJS didn't seem to fix it.

This comment has been minimized.

Show comment
Hide comment
@rundis

rundis Jul 15, 2015

Contributor

What's the purpose of cljsDeps.js ?

Contributor

rundis replied Jul 15, 2015

What's the purpose of cljsDeps.js ?

This comment has been minimized.

Show comment
Hide comment
@cldwalker

cldwalker Aug 15, 2015

Member

From code reading:

We need the compiled clojurescript in order to run clojurescript code in the background thread. As for the two dependencies, I think only cljs.reader is used but I could be wrong

Member

cldwalker replied Aug 15, 2015

From code reading:

We need the compiled clojurescript in order to run clojurescript code in the background thread. As for the two dependencies, I think only cljs.reader is used but I could be wrong

@Mouvedia

This comment has been minimized.

Show comment
Hide comment
@Mouvedia

Mouvedia Mar 7, 2015

Contributor

If you are planning to break most plugins you should maybe choose a bunch of them, fork them, fix them and move them all under a repo that you will continue to maintain.

Contributor

Mouvedia commented Mar 7, 2015

If you are planning to break most plugins you should maybe choose a bunch of them, fork them, fix them and move them all under a repo that you will continue to maintain.

@kolya-ay

This comment has been minimized.

Show comment
Hide comment
@kolya-ay

kolya-ay Mar 7, 2015

The core as it is still requires much work to dissipate the force to the foreign plugins maintaining. I propose to extend plugin metadata so that plugin's author could specify supported LT versions. Something like npm's engine, may be..

kolya-ay commented Mar 7, 2015

The core as it is still requires much work to dissipate the force to the foreign plugins maintaining. I propose to extend plugin metadata so that plugin's author could specify supported LT versions. Something like npm's engine, may be..

@cldwalker

This comment has been minimized.

Show comment
Hide comment
@cldwalker

cldwalker Mar 12, 2015

Member

Do we want to keep the browser changes in order to fix the browser eval issues?

Do we want to keep the browser changes in order to fix the browser eval issues?

This comment has been minimized.

Show comment
Hide comment
@ibdknox

ibdknox Mar 15, 2015

Member

My fixes here were hacks that didn't really work. The next release of atom-shell has this fixed for real.

Member

ibdknox replied Mar 15, 2015

My fixes here were hacks that didn't really work. The next release of atom-shell has this fixed for real.

This comment has been minimized.

Show comment
Hide comment
@Announcement

Announcement Mar 15, 2015

ibdknox and others added some commits Mar 21, 2015

latest atom-shell fixes file:// url issues in webviews, when you move…
… a browser tab it needs to reconncet to devtools, and we need to let people know about moves

Signed-off-by: Chris Granger <ibdknox@gmail.com>
fix javascript evaling the whole file everytime
Signed-off-by: Chris Granger <ibdknox@gmail.com>
add focus event, prevent navigation when files are dropped
Signed-off-by: Chris Granger <ibdknox@gmail.com>
fix css eval in browser
Signed-off-by: Chris Granger <ibdknox@gmail.com>
fix removing old css injections
Signed-off-by: Chris Granger <ibdknox@gmail.com>
Update build scripts for atom-shell name change to electron
Github has changed the name of atom-shell to electron as per
electron/electron#1389
Realised there were other references to atom-shell to update
It seems though that the download-electron grunt task seems to create an
Atom.app package. Was expecting it to be Electron.app and raised an issue
for them here electron-archive/grunt-download-electron#30

NOTE: I haven't tested this on Linux or Windows
@kenny-evitt

This comment has been minimized.

Show comment
Hide comment
@kenny-evitt

kenny-evitt May 28, 2015

Contributor

@one-more-minute it seems like the "Windows issues" and "Linux issues" checklist items should be considered complete too. I'm having trouble building on Windows myself but otherwise I think we should consider this done.

I think we should punt on proxy support for now.

I'm not sure about the last item – "Updates to LT should be driven by main.js so that they can be guaranteed to only happen once". Given the existing comments for this, I think we should punt on this for now too.

@cldwalker @ibdknox @one-more-minute – if possible, it would be great if builds could be released as an alpha version for the rest of the community to test.

Contributor

kenny-evitt commented May 28, 2015

@one-more-minute it seems like the "Windows issues" and "Linux issues" checklist items should be considered complete too. I'm having trouble building on Windows myself but otherwise I think we should consider this done.

I think we should punt on proxy support for now.

I'm not sure about the last item – "Updates to LT should be driven by main.js so that they can be guaranteed to only happen once". Given the existing comments for this, I think we should punt on this for now too.

@cldwalker @ibdknox @one-more-minute – if possible, it would be great if builds could be released as an alpha version for the rest of the community to test.

@kenny-evitt

This comment has been minimized.

Show comment
Hide comment
@kenny-evitt

kenny-evitt Jun 14, 2015

Contributor

See #1918

Contributor

kenny-evitt commented on 7d53886 Jun 14, 2015

See #1918

@kenny-evitt

This comment has been minimized.

Show comment
Hide comment
@kenny-evitt

kenny-evitt Jun 14, 2015

Contributor

See #1918

Contributor

kenny-evitt commented on b468d5d Jun 14, 2015

See #1918

kenny-evitt added some commits Jun 14, 2015

Merge pull request #1892 from martinchooooooo/atom-shell
Update build scripts for atom-shell name change to electron
Merge pull request #1899 from miraleung/electron-name-change
Electron name change: update for Linux references
@kenny-evitt

This comment has been minimized.

Show comment
Hide comment
@kenny-evitt

kenny-evitt Jun 14, 2015

Contributor

@cldwalker @ibdknox @one-more-minute @joshuafcole @rundis

There are two items not marked complete:

  • Port proxy support (we don't have a way to test this)
  • Updates to LT should be driven by main.js so that they can be guaranteed to only happen once

Can we defer completing them before creating a new release? Can defer them for a beta release?

Contributor

kenny-evitt commented Jun 14, 2015

@cldwalker @ibdknox @one-more-minute @joshuafcole @rundis

There are two items not marked complete:

  • Port proxy support (we don't have a way to test this)
  • Updates to LT should be driven by main.js so that they can be guaranteed to only happen once

Can we defer completing them before creating a new release? Can defer them for a beta release?

rundis and others added some commits Jun 16, 2015

Merge pull request #1919 from rundis/mac_electron_fix
Fix buildscript for mac app still called Atom.app not Electron.app
Merge pull request #1925 from kenny-evitt/atom-shell-add-git-attribut…
…es-to-disable-git-autocrlf

Add Git attributes file to disable auto-CRLF conversion for all text files.
Merge pull request #1927 from rundis/nix_build_precon
Added pre-condition section for Linux build on atom-shell
Merge pull request #1930 from BenjaminVanRyseghem/atom-shell
Quotes `pwd` to support spaces in path

kenny-evitt added a commit that referenced this pull request Jul 2, 2015

Merge pull request #1756 from LightTable/atom-shell
Migrate from node-webkit to Atom Shell (Electron).

@kenny-evitt kenny-evitt merged commit 8b9b4c9 into master Jul 2, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@rundis

This comment has been minimized.

Show comment
Hide comment
@rundis

rundis Jul 15, 2015

Contributor

@ibdknox Where does this version originate from, what source was used to build it ?

Contributor

rundis commented on project.clj in 8e73f59 Jul 15, 2015

@ibdknox Where does this version originate from, what source was used to build it ?

@kenny-evitt kenny-evitt deleted the atom-shell branch Sep 28, 2015

@TheInitializer

This comment has been minimized.

Show comment
Hide comment
@TheInitializer

TheInitializer Nov 4, 2015

is there a way to port it back to nw? The performance is generally a lot better there

is there a way to port it back to nw? The performance is generally a lot better there

@kenny-evitt

This comment has been minimized.

Show comment
Hide comment
@kenny-evitt

kenny-evitt Nov 4, 2015

Contributor

@TheInitializer No need; just use an old version (there's a recent issue with the links). Or fork the code prior to the merge of this the atom-shell branch. I'm not interested in maintaining two versions of the app, nor do I suspect are any of the other commiters or contributors. But I can offer tentative, sporadic, and minimal help for anyone that is so interested.

Contributor

kenny-evitt commented Nov 4, 2015

@TheInitializer No need; just use an old version (there's a recent issue with the links). Or fork the code prior to the merge of this the atom-shell branch. I'm not interested in maintaining two versions of the app, nor do I suspect are any of the other commiters or contributors. But I can offer tentative, sporadic, and minimal help for anyone that is so interested.

@rundis

This comment has been minimized.

Show comment
Hide comment
@rundis

rundis Jan 5, 2016

Contributor

@ibdknox Do you remember why this fix was added ? I tried replacing the bencode module with the latest version (0.7.0) from npm. Couldn't notice any difference, however I guess this fix was made for a reason.

@ibdknox Do you remember why this fix was added ? I tried replacing the bencode module with the latest version (0.7.0) from npm. Couldn't notice any difference, however I guess this fix was made for a reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment