Fix prompt handling in Erlang/OTP 26, closes #12584

Author: josevalim

bin/iex

@@ -30,4 +30,4 @@ readlink_f () {
 
 SELF=$(readlink_f "$0")
 SCRIPT_PATH=$(dirname "$SELF")
-exec "$SCRIPT_PATH"/elixir --no-halt --erl "-user elixir" +iex "$@"
+exec "$SCRIPT_PATH"/elixir --no-halt --erl "-user elixir" -e ":elixir.start_iex()" +iex "$@"

bin/iex.bat

@@ -24,6 +24,6 @@ goto end
 
 :run
 if defined IEX_WITH_WERL (set __ELIXIR_IEX_FLAGS=--werl) else (set __ELIXIR_IEX_FLAGS=)
-call "%~dp0\elixir.bat" --no-halt --erl "-user elixir" +iex %__ELIXIR_IEX_FLAGS% %*
+call "%~dp0\elixir.bat" --no-halt --erl "-user elixir" -e ":elixir.start_iex()" +iex %__ELIXIR_IEX_FLAGS% %*
 :end
 endlocal

lib/elixir/src/elixir.erl

@@ -2,7 +2,7 @@
 %% private to the Elixir compiler and reserved to be used by Elixir only.
 -module(elixir).
 -behaviour(application).
--export([start_cli/0, start/0]).
+-export([start_cli/0, start/0, start_iex/0]).
 -export([start/2, stop/1, config_change/3]).
 -export([
   string_to_tokens/5, tokens_to_quoted/3, 'string_to_quoted!'/5,
@@ -192,12 +192,29 @@ start_cli() ->
   'Elixir.Kernel.CLI':main(init:get_plain_arguments()),
   elixir_config:booted().
 
+%% TODO: Delete prim_tty branches and -user on Erlang/OTP 26.
 start() ->
-  case init:get_argument(elixir_root) of
-    {ok, [[Root]]} -> code:add_patha(Root ++ "/iex/ebin");
-    _ -> ok
-  end,
-  'Elixir.IEx.CLI':main().
+  case code:ensure_loaded(prim_tty) of
+    {module, _} ->
+      user_drv:start(#{initial_shell => noshell});
+    {error, _} ->
+      case init:get_argument(elixir_root) of
+        {ok, [[Root]]} -> code:add_patha(Root ++ "/iex/ebin");
+        _ -> ok
+      end,
+      'Elixir.IEx.CLI':deprecated()
+  end.
+start_iex() ->
+  case code:ensure_loaded(prim_tty) of
+    {module, _} ->
+      spawn(fun() ->
+        elixir_config:wait_until_booted(),
+        (shell:whereis() =:= undefined) andalso 'Elixir.IEx':cli()
+      end);
+
+    {error, _} ->
+      ok
+  end.
 
 %% EVAL HOOKS
 

lib/iex/lib/iex.ex

@@ -848,22 +848,101 @@ defmodule IEx do
   @spec break!(module, atom, arity, non_neg_integer) :: IEx.Pry.id()
   defdelegate break!(module, function, arity, stops \\ 1), to: IEx.Pry
 
-  ## Callbacks
+  @doc false
+  def dont_display_result, do: :"do not show this result in output"
+
+  ## CLI
+
+  @compile {:no_warn_undefined, {:shell, :start_interactive, 1}}
 
   # This is a callback invoked by Erlang shell utilities
-  # when someone presses Ctrl+G and adds 's Elixir.IEx'.
+  # when someone presses Ctrl+G and adds `s 'Elixir.IEx'`.
   @doc false
   def start(opts \\ [], mfa \\ {IEx, :dont_display_result, []}) do
-    spawn(fn ->
-      {:ok, _} = Application.ensure_all_started(:elixir)
-      System.wait_until_booted()
-      :ok = :io.setopts(binary: true, encoding: :unicode)
-      {:ok, _} = Application.ensure_all_started(:iex)
-      _ = for fun <- Enum.reverse(after_spawn()), do: fun.()
-      IEx.Server.run_from_shell(opts, mfa)
-    end)
+    # TODO: Keep only this branch, delete optional args and mfa,
+    # and delete IEx.Server.run_from_shell/2 on Erlang/OTP 26+
+    if Code.ensure_loaded?(:prim_tty) do
+      spawn(fn ->
+        {:ok, _} = Application.ensure_all_started(:iex)
+        _ = for fun <- Enum.reverse(after_spawn()), do: fun.()
+        IEx.Server.run([register: false] ++ opts)
+      end)
+    else
+      spawn(fn ->
+        {:ok, _} = Application.ensure_all_started(:elixir)
+        System.wait_until_booted()
+        :ok = :io.setopts(binary: true, encoding: :unicode)
+        {:ok, _} = Application.ensure_all_started(:iex)
+        _ = for fun <- Enum.reverse(after_spawn()), do: fun.()
+        IEx.Server.run_from_shell(opts, mfa)
+      end)
+    end
   end
 
+  # Manual tests for changing the CLI boot.
+  #
+  # 1. In some situations, we cannot read inputs as IEx boots:
+  #
+  #      $ iex -e ":io.get_line(:foo)"
+  #
+  # 2. In some situations, connecting to a remote node via --remsh
+  #    is not possible. This can be tested by starting two IEx nodes:
+  #
+  #      $ iex --sname foo
+  #      $ iex --sname bar --remsh foo
+  #
+  # 3. When still using --remsh, we need to guarantee the arguments
+  #    are processed on the local node and not the remote one. For such,
+  #    one can replace the last line above by:
+  #
+  #      $ iex --sname bar --remsh foo -e 'IO.inspect node()'
+  #
+  #    And verify that the local node name is printed.
+  #
+  # 4. Finally, in some other circumstances, printing messages may become
+  #    borked. This can be verified with:
+  #
+  #      $ iex -e ":logger.info('foo~nbar', [])"
+  #
+  # By the time those instructions have been written, all tests above pass.
   @doc false
-  def dont_display_result, do: :"do not show this result in output"
+  def cli(opts \\ []) do
+    args = :init.get_plain_arguments()
+    opts = opts ++ [dot_iex_path: find_dot_iex(args), on_eof: :halt]
+
+    ref = make_ref()
+    mfa = {__MODULE__, :__cli__, [self(), ref, opts]}
+
+    shell =
+      if remote = get_remsh(args) do
+        {:remote, remote, mfa}
+      else
+        mfa
+      end
+
+    :ok = :shell.start_interactive(shell)
+
+    receive do
+      {^ref, shell} -> shell
+    after
+      15_000 ->
+        IO.puts(:stderr, "Could not start the shell after 15 seconds, aborting...")
+        System.halt(1)
+    end
+  end
+
+  @doc false
+  def __cli__(parent, ref, opts) do
+    pid = start(opts)
+    send(parent, {ref, pid})
+    pid
+  end
+
+  defp find_dot_iex([~c"--dot-iex", h | _]), do: List.to_string(h)
+  defp find_dot_iex([_ | t]), do: find_dot_iex(t)
+  defp find_dot_iex([]), do: nil
+
+  defp get_remsh([~c"--remsh", h | _]), do: h
+  defp get_remsh([_ | t]), do: get_remsh(t)
+  defp get_remsh([]), do: nil
 end

lib/iex/lib/iex/broker.ex

@@ -13,7 +13,7 @@ defmodule IEx.Broker do
   Finds the IEx server.
   """
   @spec shell :: shell()
-  # TODO: Use shell:whereis_shell() from Erlang/OTP 26+.
+  # TODO: Use shell:whereis() from Erlang/OTP 26+.
   def shell() do
     if user = Process.whereis(:user) do
       if user_drv = get_from_dict(user, :user_drv) do
@@ -75,19 +75,50 @@ defmodule IEx.Broker do
     GenServer.call(broker_pid, {:refuse, take_ref})
   end
 
+  @doc """
+  Asks to IO if we want to take over.
+  """
+  def take_over?(location, whereami, opts) do
+    evaluator = opts[:evaluator] || self()
+    message = "Request to pry #{inspect(evaluator)} at #{location}#{whereami}"
+    interrupt = IEx.color(:eval_interrupt, "#{message}\nAllow? [Yn] ")
+    yes?(IO.gets(:stdio, interrupt))
+  end
+
+  defp yes?(string) do
+    is_binary(string) and String.trim(string) in ["", "y", "Y", "yes", "YES", "Yes"]
+  end
+
   @doc """
   Client requests a takeover.
   """
   @spec take_over(binary, iodata, keyword) ::
           {:ok, server :: pid, group_leader :: pid, counter :: integer}
           | {:error, :no_iex | :refused}
   def take_over(location, whereami, opts) do
-    case GenServer.whereis(@name) do
-      nil ->
-        {:error, :no_iex}
+    case take_over_existing(location, whereami, opts) do
+      {:error, :no_iex} ->
+        cond do
+          # TODO: Remove this check on Erlang/OTP 26+ and {:error, :no_iex}
+          not Code.ensure_loaded?(:prim_tty) ->
+            {:error, :no_iex}
+
+          take_over?(location, whereami, opts) ->
+            {:ok, IEx.cli(opts), Process.group_leader(), 1}
+
+          true ->
+            {:error, :refused}
+        end
+
+      other ->
+        other
+    end
+  end
 
-      _pid ->
-        GenServer.call(@name, {:take_over, location, whereami, opts}, :infinity)
+  defp take_over_existing(location, whereami, opts) do
+    case GenServer.whereis(@name) do
+      nil -> {:error, :no_iex}
+      _pid -> GenServer.call(@name, {:take_over, location, whereami, opts}, :infinity)
     end
   end
 

lib/iex/lib/iex/cli.ex

@@ -1,69 +1,26 @@
-# IEx sets the Erlang user to be IEx.CLI via the command line.
-# While this works most of the time, there are some problems
-# that may happen depending on how the booting process goes.
-# Those problems need to be manually tested in case changes are
-# done to this file.
-#
-# 1. In some situations, printing something before the shell
-#    starts becomes very slow. To verify this feature, just
-#    get an existing project and run:
-#
-#      $ mix clean
-#      $ iex -S mix
-#
-#    If the printing of data is slower than usual. This particular
-#    bug has arisen;
-#
-# 2. In some situations, connecting to a remote node via --remsh
-#    is not possible. This can be tested by starting two IEx nodes:
-#
-#      $ iex --sname foo
-#      $ iex --sname bar --remsh foo@localhost
-#
-# 3. When still using --remsh, we need to guarantee the arguments
-#    are processed on the local node and not the remote one. For such,
-#    one can replace the last line above by:
-#
-#      $ iex --sname bar --remsh foo@localhost -e 'IO.inspect node()'
-#
-#    And verify that the local node name is printed.
-#
-# 4. Finally, in some other circumstances, printing messages may become
-#    borked. This can be verified with:
-#
-#      $ iex -e ":logger.info('foo~nbar', [])"
-#
-# By the time those instructions have been written, all tests above pass.
+# Remove this whole module on Erlang/OTP 26+.
 defmodule IEx.CLI do
   @moduledoc false
 
   @compile {:no_warn_undefined, {:user, :start, 0}}
 
-  def main do
-    # TODO: Keep only the first branch and remove the usage
-    # of -user callback altogether on Erlang/OTP 26+ by using
-    # --eval and :shell.start_interactive(new_tty_args())
-    cond do
-      Code.ensure_loaded?(:prim_tty) ->
-        :user_drv.start(%{initial_shell: new_tty_args()})
-
-      tty_works?() ->
-        :user_drv.start([:"tty_sl -c -e", old_tty_args()])
-
-      true ->
-        if get_remsh(:init.get_plain_arguments()) do
-          IO.puts(
-            :stderr,
-            "warning: the --remsh option will be ignored because IEx is running on limited shell"
-          )
-        end
+  def deprecated do
+    if tty_works?() do
+      :user_drv.start([:"tty_sl -c -e", old_tty_args()])
+    else
+      if get_remsh(:init.get_plain_arguments()) do
+        IO.puts(
+          :stderr,
+          "warning: the --remsh option will be ignored because IEx is running on limited shell"
+        )
+      end
 
-        :user.start()
+      :user.start()
 
-        # IEx.Broker is capable of considering all groups under user_drv but
-        # when we use :user.start(), we need to explicitly register it instead.
-        # If we don't register, pry doesn't work.
-        IEx.start([register: true] ++ options())
+      # IEx.Broker is capable of considering all groups under user_drv but
+      # when we use :user.start(), we need to explicitly register it instead.
+      # If we don't register, pry doesn't work.
+      IEx.start([register: true] ++ options())
     end
   end
 
@@ -84,14 +41,6 @@ defmodule IEx.CLI do
     end
   end
 
-  defp new_tty_args do
-    if remote = get_remsh(:init.get_plain_arguments()) do
-      {:remote, remote, remote_start_mfa()}
-    else
-      local_start_mfa()
-    end
-  end
-
   defp old_tty_args do
     if remote = get_remsh(:init.get_plain_arguments()) do
       remote = List.to_atom(append_hostname(remote))

lib/iex/lib/iex/server.ex

@@ -27,12 +27,16 @@ defmodule IEx.Server do
     * `:env` - the `Macro.Env` used for the evaluator
     * `:binding` - an initial set of variables for the evaluator
     * `:on_eof` - if it should `:stop_evaluator` (default) or `:halt` the system
+    * `:register` - if this shell should be registered in the broker (default is `true`)
 
   """
   @doc since: "1.8.0"
   @spec run(keyword) :: :ok
   def run(opts) when is_list(opts) do
-    IEx.Broker.register(self())
+    if Keyword.get(opts, :register, true) do
+      IEx.Broker.register(self())
+    end
+
     run_without_registration(init_state(opts), opts, nil)
   end
 
@@ -289,10 +293,8 @@ defmodule IEx.Server do
   end
 
   defp take_over?(take_pid, take_ref, take_location, take_whereami, take_opts, counter) do
-    evaluator = take_opts[:evaluator]
-    message = "Request to pry #{inspect(evaluator)} at #{take_location}#{take_whereami}"
-    interrupt = IEx.color(:eval_interrupt, "#{message}\nAllow? [Yn] ")
-    take_over?(take_pid, take_ref, counter, yes?(IO.gets(:stdio, interrupt)))
+    answer = IEx.Broker.take_over?(take_location, take_whereami, take_opts)
+    take_over?(take_pid, take_ref, counter, answer)
   end
 
   defp take_over?(take_pid, take_ref, counter, response) when is_boolean(response) do
@@ -309,10 +311,6 @@ defmodule IEx.Server do
     end
   end
 
-  defp yes?(string) do
-    is_binary(string) and String.trim(string) in ["", "y", "Y", "yes", "YES", "Yes"]
-  end
-
   ## State
 
   defp init_state(opts) do

lib/iex/test/iex/server_test.exs

@@ -18,12 +18,6 @@ defmodule IEx.ServerTest do
   end
 
   describe "pry" do
-    test "no sessions" do
-      assert capture_io(fn ->
-               assert IEx.pry() == {:error, :no_iex}
-             end) =~ "Is an IEx shell running?"
-    end
-
     test "inside evaluator itself" do
       assert capture_iex("require IEx; IEx.pry()") =~ "Break reached"
     end