wbond / sublime_terminal

Launch terminals from the current file or the root project folder
https://packagecontrol.io/packages/Terminal
MIT License
600 stars 117 forks source link

iterm 3 opens in new tab + the default #131

Closed ctf0 closed 8 years ago

ctf0 commented 8 years ago

currently when u fire the plugin, it will open iterm with 2 tabs like so

test

also the script needs to activate the terminal after its opened.

twolfson commented 8 years ago

Which iTerm script are you using?

twolfson commented 8 years ago

I believe this is a duplicate of #132 which has been patched via #133. Closing issue as a result

ctf0 commented 8 years ago

still exist, nothing changed :( & am using the latest iTerm2-v3

garyking commented 8 years ago

Is it possible that you have iTerm running but it just doesn't have any windows open? So it has the black dot underneath it in the Dock?

ctf0 commented 8 years ago

nop.

ctf0 commented 8 years ago

update the bringing iterm window forward now is fixed, now the double tab issue btw if there a specific option i should check/uncheck in iterm please tell me.

twolfson commented 8 years ago

Can you paste your configuration and your version of Terminal (found via Package Control: List Packages in the Command Palette)?

ctf0 commented 8 years ago

sure

{
    "terminal": "iTerm2-v3.sh"
}

test

twolfson commented 8 years ago

Okay, everything looks good. You have explained the unexpected behavior but can you explain what you expect?

ctf0 commented 8 years ago

open in one tab without the extra default :smiley:

twolfson commented 8 years ago

When you run the plugin is iTerm already open?

On Sep 2, 2016 6:07 AM, "Muah" notifications@github.com wrote:

open in one tab without the extra default 😃

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/wbond/sublime_terminal/issues/131#issuecomment-244369749, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3FWHM0J2V5n6T632xzMMJGLHhx7UwPks5qmB-KgaJpZM4JttzO .

ctf0 commented 8 years ago

@twolfson sorry for the delayed answer, no

twolfson commented 8 years ago

Alright, going to re-open this issue and mark as "help wanted" as I don't have an OS X machine to test/reproduce the issue on =/ But it sounds like the steps to reproduce are:

Expected behavior:

Actual behavior:

ctf0 commented 8 years ago

thanx

ctf0 commented 8 years ago

@twolfson currently if another windows was already opened, the plugin work as expected open a new window for the ST path, maybe u can add like a hook which check if there was no opened window, the plugin will close any tabs other than the one with the path from ST

twolfson commented 8 years ago

As I said I don't have an OS X machine so I can't do anything beyond marking this as "help wanted". Feel free to take a shot at patching the issue though :+1:

crapthings commented 8 years ago

i can confirm that if i press Ctrl+Q with iterm, there will be two tabs, one is default, and the second is opened with this terminal plugin.

if i just click close button there will be only 1 tab open correctly.

crapthings commented 8 years ago

so if you want this work. you might have to change iterm preferences

startup Don't open any windows, but i think this is not convenient.

https://www.iterm2.com/documentation-restoration.html

@ctf0

ctf0 commented 8 years ago

@crapthings am tyring to make iterm work as the default terminal work

so am not sure what exactly is the option i should change in iterm to achive that but i dont think this is related to iterm because of https://github.com/wbond/sublime_terminal/issues/131#issuecomment-249361290

crapthings commented 8 years ago

is there something like "open with external" with iterm ?

ctf0 commented 8 years ago

under which settings ?

plashenkov commented 8 years ago

It seems that this line: https://github.com/wbond/sublime_terminal/blob/master/iTerm2-v3.sh#L36 is unnecessary. When iTerm starts, it already opens a tab. If you comment the line, everything becomes as it should.

ctf0 commented 8 years ago

@plashenkov fuck yeah 💃

crapthings commented 8 years ago

so we will get this fix soon ?

twolfson commented 8 years ago

I don't have a OS X device to test on but will gladly review any PR that someone else has tested

ctf0 commented 8 years ago

@twolfson comment the line and test if u have any issues on ur machine, if all good i can submit the PR

twolfson commented 8 years ago

I don't have an OS X device so commenting the line won't really affect me as it's an OS X specific script =/

ctf0 commented 8 years ago

sweet, then i'll submit the PR

plashenkov commented 8 years ago

It looks like some mistake is above.

Look, here we get the version: https://github.com/wbond/sublime_terminal/blob/master/iTerm2-v3.sh#L7

Here we check the version is less or larger than 10.7: https://github.com/wbond/sublime_terminal/blob/master/iTerm2-v3.sh#L21

And despite the fact my version is 10.12 it acts like if it less than 10.7. This is why algorithm later goes in the wrong direction. Or maybe I missed something?

ctf0 commented 8 years ago

i will submit the PR & i will keep this opened till further notice

twolfson commented 8 years ago

It looks like that's a hold-over from iTerm.sh that @pzgz never removed =/ @pzgz Was leaving that VERSION check in intentional?

https://github.com/wbond/sublime_terminal/blob/1.14.0/iTerm.sh#L21-L30

plashenkov commented 8 years ago

Well, I do not know. If we remove this part: https://github.com/wbond/sublime_terminal/blob/master/iTerm2-v3.sh#L22-L25 and leave this part: https://github.com/wbond/sublime_terminal/blob/master/iTerm2-v3.sh#L29 then we still get double tab, and logic below this line needs corrections.

plashenkov commented 8 years ago

It's interesting, does this comparison actually work as the author of these lines was expected.

Here is simple script:

VERSION=10.12

if (( $(expr $VERSION '<' 10.7) )); then
    echo "$VERSION < 10.7"
else
    echo "$VERSION >= 10.7"
fi

10.12 < 10.7 10.8 >= 10.7 2 >= 10.7 etc.

twolfson commented 8 years ago

This has been patched by @ctf0 in #131 and released in 1.15.0. If we still have issues, then please let us know