boot2docker / osx-installer

Docker installer for Mac OS X
Apache License 2.0
1.25k stars 258 forks source link

Adding the support of iTerm2 #65

Closed jstoja closed 6 years ago

jstoja commented 9 years ago

Hello,

We discussed about it here https://github.com/boot2docker/boot2docker/issues/414 I kept the support of the default terminal, and let the commands at the top so it will be easier to change them if needed. Working on my laptop so... Try it out too :) (btw I didn't signed the commit since it's not asked for this repo ;))

ping @SvenDowideit @jlecour

jstoja commented 9 years ago

up @SvenDowideit ?

SvenDowideit commented 9 years ago

Heya, (was on holidays)

I'd love to have the common code extracted into a bash script that the installer then puts into usr/local/bin and then have this applet call it (both to de-dupe, and to make it possible to version control the changes better)

if you can do that, great, otherwise, it'll be a while before i've caught up enough to test this

@tianon wanna review it?

tianon commented 9 years ago

I'm a little confused as to why we get "Binary file not shown" from GitHub - isn't this a script?

tianon commented 9 years ago

(but I definitely agree with @SvenDowideit that common stuff should be extracted to simple bash scripts wherever possible)

jstoja commented 9 years ago

I will modify this this weekend ;) Totally agreed too. The AppleScripts seem to be some strange scripts that Github can't display because it's proprietary. I'll notify you when I push it ;)

jstoja commented 9 years ago

I added a script /usr/local/share/boot2docker called start.sh. It's located in a package boot2dockerutils.pkg because I wanted to put it in /usr/local/share/boot2docker and do not wanted to put it with the iso. If you have some things to correct, let me know :)

SvenDowideit commented 9 years ago

awesome! I like it - and the uninstall script already cleans that up :)

LGTM

@tianon this needs testing on an OSX that does not have iterm installed - thats what tripped us up last time :/

jstoja commented 9 years ago

Just tested it on a laptop at work. The only problem there is I guess is that it's launching 2 terms instead of one. I'll investigate this soon.

jstoja commented 9 years ago

I just tested it on a mac without iterm2, it's now working and opening 1window.

SvenDowideit commented 9 years ago

sorry, this will need to wait til 1.4 - we're flat out with what we have already :/

jstoja commented 9 years ago

Not a problem :) That let us more time to get it polished and add more features ;)

artiom commented 9 years ago

@jstoja maybe I'm looking at a different file, however when I open main.scpt without iTerm present on my system I get a prompt to find it. I had similar problem when I tried to add support for iTerm as well (https://github.com/boot2docker/osx-installer/pull/50).

The problematic code is:

tell application "iTerm"
            activate
            set myterm to (make new terminal)
            tell myterm
                launch session "Default Session"
                tell the last session
                    set name to "Boot2Docker for OSX"
                    write text b2d_launch_script
                end tell
            end tell
        end tell

AppleScript tries to "validate" iTerm calls anyway and at this stage it throws prompt to find iTerm if it doesn't exists.

The only 100% solution that I could find is to make iTerm calls a string and evaluate them in runtime if iTerm is installed. This is what I did here: https://github.com/artiom/osx-installer/tree/iterm2_support

I was going to create PR before I noticed this PR, which has a better approach (bash script with common calls).

In addition, I didn't see option to select terminal app under which boot2docker will run (if iTerm is installed). You can grab this from my branch as well (if you want to).

jstoja commented 9 years ago

I'm taking note of this ! Thank you very much ;) I'll update this PR and use your remarks !

jstoja commented 9 years ago

@artiom Your version was very clear and was working. I merged it with my version and pushed. Since the PR is a bit old, updated the repo so we're closest the HEAD.

If some people can try it too, both the iterm and terminal version are working on my laptop. If you want to remove the preference, you can remove the file ~/.boot2docker/preferences.scpt.

Have nice christmas time !

SvenDowideit commented 9 years ago

mmm, @artiom @jstoja it still isn't working for me.

If I uninstall iTerm, then running this new script (or even just opening it in the applescript editor) will pop up a find iTerm dialog window.

argh?

ah, yup, the tell application "iTerm.app" causes things to go off the rails.

I'm going to guess the solution is to extract the run_iterm() out into its own script.

artiom commented 9 years ago

@jstoja if you look at my version of the main.scpt: https://github.com/artiom/osx-installer/blob/iterm2_support/mpkg/boot2docker.app/Contents/Resources/Scripts/main.scpt

It loads the whole run_iterm into a string itermCmd and than executes it through run script itermCmd. In this PR it still tries to communicate directly with iTerm though tell application "iTerm.app" as @SvenDowideit mentioned above. Unfortunately, this brings iTerm search prompt.

Just to confirm what @SvenDowideit discovered as well, I get application search prompt too when iTerm is not installed.

tianon commented 6 years ago

Sorry, this repository is long-since deprecated in favor of Docker Toolbox (whose usage is now also discouraged in favor of Docker for Windows and Docker for Mac).