achiurizo / consular

Terminal automation
http://rdoc.info/github/achiu/consular/master/file/README.md
MIT License
813 stars 46 forks source link

Use ordered hashes to further fix 1.8 compat issues #90

Closed mcmire closed 12 years ago

mcmire commented 12 years ago

I have this Termfile:

window do
  tab :name => "guard" do
    run "j campfire; bundle exec guard"
  end

  tab :name => "console" do
    run "j campfire; d"
  end
end

I have noticed all of a sudden that when I run terminitor, the "guard" tab will open in another window, but the "console" tab will open in the current window. If I patch AbstractCore#process! to tell me what the windows hash is, it is this:

{:term_windows=>
  {"window1"=>
    {:tabs=>
      {"default"=>{:commands=>[]},
       "tab1"=>
        {:commands=>["j campfire; bundle exec guard"],
         :options=>{:name=>"guard"}}}},
   "default"=>
    {:tabs=>
      {"default"=>{:commands=>[]},
       "tab1"=>{:commands=>["j campfire; d"], :options=>{:name=>"console"}}}}}}

I have tracked this down to #last_open_window in AbstractCore. It appears that under Ruby 1.8, this does not always return the last open window but in fact could return any of the windows. This is because it uses keys.last to pull the last window from the windows hash. However, since hashes in 1.8 are of course unordered, this is non-deterministic.

I have fixed this by copying the OrderedHash class from ActiveSupport and using that to store windows and tabs. Coincidentally, the tests for DSL, which currently fail on 1.8, now pass (and if there were any other files that failed, they now pass too).

mcmire commented 12 years ago

I also just tried my termfile with these changes and it fixes the issue -- all tabs are opening in the new window.

achiurizo commented 12 years ago

I actually fixed this issue while i was refactoring here:

. The only thing is the current refactored DSL class expects name tabs to be a hash option, not a first param. Tests is passing again for 1.8 and 1.9 though I haven't tested with it with the cores( as I am also cleaning that up). Would you mind trying the 1.x branch and seeing if that could work?

mcmire commented 12 years ago

It does seem to work, but I use iTerm, so I can't use it day-to-day just yet. Would you accept patches for consular-osx?

achiurizo commented 12 years ago

I'm actually working on a consular-iterm, unless someone beats me to the punch. But yes I would accept patches to consular-osx. I'd prefer though if its iTerm related if it be its own gem/core.

mcmire commented 12 years ago

Oh, nice. No worries then, I will wait until you have that out.

achiurizo commented 12 years ago

@mcmire you can check out consular-iterm with consular and let me know if this works for you. The issue here should be fixed and address. Otherise open an issue over there if it still persists. Thanks!