From 46d9bbcddfcd509558437e4aa9d744814755a824 Mon Sep 17 00:00:00 2001
From: Amos L King <amos@binarynoggin.com>
Date: Fri, 21 Apr 2023 16:14:44 -0500
Subject: [PATCH] Warns missing `handle_input/3` on `request_input/2`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With all the different handlers for events, inputs, and messages, it
can be simple to pick the wrong one when implementing. This leads to
bugs that are difficult to find.

I didn’t want development workflows or older projects to break, so I
thought a warning would be an excellent solution to reduce the time
spent debugging but still allow current projects to continue to work
with this enhancement. This means that it is still possible to implement
the wrong callback, but hopefully, the warning will point the developer
in the right direction and save minutes, if not hours, of debugging.
---
 lib/scenic/scene.ex                  |  5 ++++-
 test/scenic/scene_test.exs           | 33 ++++++++++++++++++++++++++++
 test/scenic/view_port/input_test.exs |  4 ++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/lib/scenic/scene.ex b/lib/scenic/scene.ex
index 3ca5d5bf..01f7bc75 100644
--- a/lib/scenic/scene.ex
+++ b/lib/scenic/scene.ex
@@ -722,7 +722,10 @@ defmodule Scenic.Scene do
   def request_input(scene, input_class)
   def request_input(scene, input) when is_atom(input), do: request_input(scene, [input])
 
-  def request_input(%Scene{viewport: vp, pid: pid}, inputs) when is_list(inputs) do
+  def request_input(%Scene{viewport: vp, pid: pid, module: module}, inputs) when is_list(inputs) do
+    unless Kernel.function_exported?(module, :handle_input, 3) do
+      Logger.warn("Requesting input for #{inspect inputs} - #{module}.handle_input/3 not implemented")
+    end
     ViewPort.Input.request(vp, inputs, pid: pid)
   end
 
diff --git a/test/scenic/scene_test.exs b/test/scenic/scene_test.exs
index 6c8b7dcd..606c533c 100644
--- a/test/scenic/scene_test.exs
+++ b/test/scenic/scene_test.exs
@@ -13,6 +13,8 @@ defmodule Scenic.SceneTest do
   alias Scenic.Script
   import Scenic.Components
 
+  import ExUnit.CaptureLog
+
   @root_id ViewPort.root_id()
 
   # import IEx
@@ -68,6 +70,23 @@ defmodule Scenic.SceneTest do
       scene = assign(scene, pid: pid)
       {:ok, scene}
     end
+
+    @impl Scenic.Scene
+    def handle_input(input, id, %{assigns: %{pid: pid}} = scene) do
+      send(pid, {:input_test, input, id})
+      {:noreply, scene}
+    end
+  end
+
+  defmodule TestSceneNoInputHandler do
+    use Scenic.Scene
+
+    @impl Scenic.Scene
+    def init(scene, pid, _opts) do
+      Process.send(pid, {:up, scene}, [])
+      scene = assign(scene, pid: pid)
+      {:ok, scene}
+    end
   end
 
   @codepoint {:codepoint, {"k", []}}
@@ -403,6 +422,20 @@ defmodule Scenic.SceneTest do
              {:ok, [:cursor_pos, :cursor_button, :codepoint]}
   end
 
+  test "request_input warns when handle_input is not implements" do
+    Scenic.Test.ViewPort.start({TestSceneNoInputHandler, self()})
+
+    scene =
+      receive do
+        {:up, scene} -> scene
+      end
+
+    assert capture_log(fn ->
+      Scenic.Scene.request_input(scene, [:key])
+    end)
+    =~ "handle_input/3 not implemented"
+  end
+
   test "unrequest_input works", %{scene: scene} do
     Scenic.Scene.request_input(scene, :cursor_button)
 
diff --git a/test/scenic/view_port/input_test.exs b/test/scenic/view_port/input_test.exs
index c9522a94..7f090350 100644
--- a/test/scenic/view_port/input_test.exs
+++ b/test/scenic/view_port/input_test.exs
@@ -27,6 +27,10 @@ defmodule Scenic.ViewPort.InputTest do
     # def handle_input( input, %{assigns: %{pid: pid}} = scene ) do
     #   Process.send( pid, {:test_input, input}, [] )
     # end
+
+    def handle_input(_, _, scene) do
+      {:noreply, scene}
+    end
   end
 
   setup do