Hello world example fails

[usoban]

Hello,

first of all, thanks for your effort with this library. Really appreciate it.

Now onto the issue: running either the basic hello world example or event viewer example fails for me with the following message:

(exit) exited in: GenServer.call(ExTermbox.EventManager, {:subscribe, #PID<0.74.0>}, 5000) (EXIT) no process: the process is not alive or there's no process currently associated with the given name, possibly because its application isn't started (elixir) lib/gen_server.ex:914: GenServer.call/3 examples/event_viewer.exs:14: EventViewer.run/0 (elixir) lib/code.ex:767: Code.require_file/2 (mix) lib/mix/tasks/run.ex:146: Mix.Tasks.Run.run/5

I've traced the issue to these two lines:

{:ok, _pid} = EventManager.start_link()
:ok = EventManager.subscribe(self())

Since EventManager.subscribe/2 accepts two arguments, the first one being PID of the EventManager, and we're passing it PID of the current process (which is iex shell in our case), I've modified these two lines just a little bit to get them working:

{:ok, em_pid} = EventManager.start_link()
:ok = EventManager.subscribe(em_pid, self())

I see the EventManager.subscribe/2 source has been modified within the last month, most importantly, the first argument was repurposed to be EventManager PID and has a default value of __MODULE__. IMO that argument should be put second, since its breaking backwards compatibility (and examples).

[ndreynolds]

@usoban Thank you very much for the bug report! I've made a small fix in ee900a1 that should get everything working again.

So I had made some changes to EventManager.start_link/1 in order to support overriding the termbox bindings module, but I see that I accidentally broke passing a default server name.

EventManager.subscribe/2 should still be backwards compatible, as it's possible to use either of the following forms again:

EventManager.subscribe(em_pid, self())

EventManager.subscribe(self())

So I think it was only a problem of the name not being set. But let me know if you think there's still any backwards incompatibility.

I'm also looking for a way to automate testing these examples so that this doesn't happen again.

[usoban]

@ndreynolds Thank you very much for such a fast response. I've tried with current master and it works fine now :)

I was under impression that it is overriding the first argument with subscriber's PID. I was not aware Elixir will automatically apply the default value for the first argument, and put the only argument in second place, as demonstrated by:

defmodule TestDefault do
  def test(a \\ "default", b) do
    IO.puts "value of a is #{a}"
    IO.puts "value of b is #{b}"
  end
end

TestDefault.test("bar")

>> value of a is default
>> value of b is bar

So I also learned something along the way. Neat! :)

If you need any help with tests, let me know. Since I plan to use ratatouille library extensively, I'd be glad to contribute something back.

Cheers!

[ndreynolds]

@usoban So far I came up with https://github.com/ndreynolds/ex_termbox/blob/ca2cc84fcc39f42d9972ebc36145dfe5649983fc/test/integration/examples_test.exs to test the examples.

It works, but it's also a bit hacky because of termbox taking over the window and needing to clean up what the examples start in on_exit. (You can run with mix test.integration if you want to try it.)

Thanks for the offer. Maybe you have some ideas for improvements, or want to add similar tests to ratatouille. Any help is very welcome. I'd also really appreciate any feedback you have on what's confusing, missing, broken, etc. in the libraries when you use them. You can find my email address in my profile :)