Enable codeintel in the IDE by etiennewan · Pull Request #98 · JanitorTechnology/dockerfiles

etiennewan · 2017-10-08T21:17:19Z

I experienced a popup in the IDE asking if I wanted to install and enable codeintel.
This PR is based on https://github.com/c9/c9.ide.language.codeintel and on https://github.com/c9/c9.ide.language.codeintel/blob/435e0840663ee5e7fecaabfd3daa865eb2113e4c/codeintel.js#L149.

jankeromnes

🎉 Thanks a lot for this pull request! 🎉 😄

I experienced a popup in the IDE asking if I wanted to install and enable codeintel.

Oh oh, this sounds like a bug. Are you sure that codeintel isn't already installed? I think it's supposed to be a "core plugin" of Cloud9, i.e. pre-installed with Cloud9 SDK.

Do you have a way to systematically trigger this pop-up, in order for me to reproduce this issue? I have never seen it myself, nor have I heard other users mentioning it.

Maybe you were trying to unlock some advanced Cloud9 feature? If so, I'd be super happy to help enable whatever you're trying to achieve 😄

jankeromnes · 2017-10-09T09:57:53Z

ubuntu-dev/ubuntu-dev.dockerfile

+# Install dependencies to enable codeintel (https://github.com/c9/c9.ide.language.codeintel)
+RUN easy_install pip
+ && pip install -U pip
+ && pip install -U virtualenv


Nit: This will no longer be necessary when #97 is merged (it will already have the latest pip and virtualenv).

removed considering #97

jankeromnes · 2017-10-09T09:59:01Z

ubuntu-dev/ubuntu-dev.dockerfile

+ && virtualenv --python=python2 $HOME/.c9/python2
+ && source $HOME/.c9/python2/bin/activate
+ && apt-get update
+ && apt-get install python-dev


Nit: This shouldn't be necessary. If you need python-dev to be installed in the ubuntu-dev image, please add it to the list of installed packages at the top of this dockerfile.

moved python-dev to the top of file, I think it is indeed needed, but I'm not 100% sure

jankeromnes · 2017-10-09T10:00:30Z

ubuntu-dev/ubuntu-dev.dockerfile

+ && pip download -d /tmp/codeintel codeintel==0.9.3
+ && cd /tmp/codeintel
+ && tar xf CodeIntel-0.9.3.tar.gz
+ && mv CodeIntel-0.9.3/SilverCity CodeIntel-0.9.3/silvercity


Interesting, why are you renaming SilverCity to silvercity?

I don't know much, I have to check https://github.com/c9/c9.ide.language.codeintel, that's what they advocate to do

jankeromnes · 2017-10-09T10:03:02Z

ubuntu-dev/ubuntu-dev.dockerfile

+ && tar xf CodeIntel-0.9.3.tar.gz
+ && mv CodeIntel-0.9.3/SilverCity CodeIntel-0.9.3/silvercity
+ && tar czf CodeIntel-0.9.3.tar.gz CodeIntel-0.9.3
+ && pip install -U --no-index --find-links=/tmp/codeintel codeintel


Nit: Please delete the temporary /tmp/codeintel folder once you no longer need it. (And if some files need to be kept around for the installation to work, please move them somewhere under /home/user, e.g. as dotfiles or even better close to where it will be used, e.g. under a new folder in /home/user/.c9 as we do with noVNC).

They advocate to work under /tmp/ for this part, I assume pip does all the work, and nothing is necessary afterwards. done

All these steps look like a very complicated way to do pip install codeintel==0.9.3. Wouldn't such a simple command work as well?

jankeromnes · 2017-10-09T10:03:59Z

ubuntu-dev/workspace-janitor.js

          p.settings.user.terminal['@scrollback'] = 10000;
+
+          if (!p.settings.project) {
+            p.settings.user = {};


Nit: You probably mean to create p.settings.project here, as opposed to p.settings.user.

jankeromnes · 2017-10-09T10:04:18Z

ubuntu-dev/workspace-janitor.js

+          }
+
+          if (!p.settings.project.codeintel) {
+            p.settings.user.codeintel = {};


Nit: Same as above, s/user/project/.

etiennewan · 2017-10-09T15:48:27Z

Last commit fixes my mistakes and your nits. This should be working, but better test twice!
I was talking about the popup which is triggered when the IDE thinks you're looking for snippets or autocompletion. One way to trigger it (from the IDE of any actual or fresh-spawned container), is to create and save a PHP file, then type '<?php', then '$'. The popup should show up asking you if you want to install codeintel.

I need more testing, the codeintel server doesn't appear to be running...

jankeromnes

Thanks for updating this pull request! 😄

I noticed that the codeintel repository is already checked-out in c9/core/plugins: https://github.com/c9/core/tree/master/plugins/c9.ide.language.codeintel so I find it strange that c9/core/scripts/install-sdk.sh doesn't already install such external dependencies. Oh well, at least we'll be doing that for Janitor containers (thanks!)

jankeromnes · 2017-10-13T14:38:53Z

ubuntu-dev/ubuntu-dev.dockerfile

  valgrind \
  x11vnc \
  xvfb \
+  python-dev \


Nit: Please try to keep the list of packages in alphabetical order.

jankeromnes · 2017-10-13T14:39:48Z

ubuntu-dev/ubuntu-dev.dockerfile

 && node ./utils/use_require.js --as commonjs --with-app
+
+# Install dependencies to enable codeintel (https://github.com/c9/c9.ide.language.codeintel)
+RUN virtualenv --python=python2 $HOME/.c9/python2


Nit: Please hardcode /home/user instead of using the $HOME variable (just to be extra safe).

Nit: All these lines need to be terminated by a \ (otherwise docker build will complain about && not being a valid Dockerfile instruction).

jankeromnes · 2017-10-13T14:44:05Z

ubuntu-dev/ubuntu-dev.dockerfile

+ && tar xf CodeIntel-0.9.3.tar.gz
+ && mv CodeIntel-0.9.3/SilverCity CodeIntel-0.9.3/silvercity
+ && tar czf CodeIntel-0.9.3.tar.gz CodeIntel-0.9.3
+ && pip install -U --no-index --find-links=/tmp/codeintel codeintel


All these steps look like a very complicated way to do pip install codeintel==0.9.3. Wouldn't such a simple command work as well?

jankeromnes · 2017-10-13T14:45:26Z

ubuntu-dev/workspace-janitor.js

          // Use a longer scrollback for the Terminal.
          p.settings.user.terminal['@scrollback'] = 10000;
+
+          if (!p.settings.project) {


Nit: Please add a comment similar to "Tweak the default user settings" above (ideally with a link to a file or list of every available project setting).

etiennewan added 2 commits October 8, 2017 23:06

[ubuntu-dev] install dependencies to enable codeintel

187d8f2

[ubuntu-dev] dismiss the codeintel popup

66a6b77

etiennewan requested a review from jankeromnes October 8, 2017 21:17

jankeromnes reviewed Oct 9, 2017

View reviewed changes

[ubuntu-dev] codeintel: fix and cleanup

5358b24

jankeromnes reviewed Oct 13, 2017

View reviewed changes

Conversation

etiennewan commented Oct 8, 2017

Uh oh!

jankeromnes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etiennewan commented Oct 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jankeromnes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jankeromnes Oct 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

etiennewan commented Oct 9, 2017 •

edited

Loading

jankeromnes Oct 13, 2017 •

edited

Loading