Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable codeintel in the IDE #98

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions ubuntu-dev/ubuntu-dev.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ RUN __LLVM_VERSION__="5.0" \
valgrind \
x11vnc \
xvfb \
python-dev \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

&& mkdir /var/run/sshd \
&& pip install --upgrade pip \
&& pip install --upgrade virtualenv \
Expand Down Expand Up @@ -201,6 +202,18 @@ RUN git clone https://github.com/kanaka/noVNC /home/user/.novnc/ \
&& cd /home/user/.novnc \
&& npm install \
&& 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@jankeromnes jankeromnes Oct 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

&& source $HOME/.c9/python2/bin/activate
&& mkdir /tmp/codeintel
&& 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, why are you renaming SilverCity to silvercity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

&& tar czf CodeIntel-0.9.3.tar.gz CodeIntel-0.9.3
&& pip install -U --no-index --find-links=/tmp/codeintel codeintel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

&& rm -rf /tmp/codeintel

# Install the latest Cloud9 SDK with some useful IDE plugins.
RUN git clone https://github.com/c9/core.git /home/user/.c9sdk \
Expand Down
15 changes: 15 additions & 0 deletions ubuntu-dev/workspace-janitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,21 @@ module.exports = function (options) {

// Use a longer scrollback for the Terminal.
p.settings.user.terminal['@scrollback'] = 10000;

if (!p.settings.project) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

p.settings.project = {};
}

if (typeof p.settings.project === 'string') {
p.settings.project = JSON.parse(p.settings.project);
}

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

// Dismiss the codeintel popup because it's installed
p.settings.project.codeintel['@dismiss_installer'] = true;
}
break;
}
Expand Down