From a52ef163d6a8beb9f322b286d2ff61ec60c86727 Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 1 Oct 2026 15:37:01 +0300 Subject: [PATCH 1/6] Reply to close with session-closed vim-fireplace only forgets a session once it sees session-closed, and nREPL replies with it too. Closing a session that's already gone (or no session at all) gets session-closed as well, like Babashka, nbb and ClojureCLR do, so clients that wait for it don't loop. The session now leaves the registry before it replies. Otherwise a request sent right after session-closed could still be routed to the session while it shut down and never get an answer. --- dialtone/src/dialtone_conn.erl | 8 +++++--- dialtone/src/dialtone_session.erl | 10 +++++++++- dialtone/src/dialtone_sessions.erl | 13 ++++++++++++- dialtone/test/dialtone_SUITE.erl | 21 ++++++++++++++++++--- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/dialtone/src/dialtone_conn.erl b/dialtone/src/dialtone_conn.erl index f6da326..db220ed 100644 --- a/dialtone/src/dialtone_conn.erl +++ b/dialtone/src/dialtone_conn.erl @@ -156,20 +156,22 @@ ls_sessions(Req, State) -> State). %% close is idempotent: closing an unknown (or absent) session still answers -%% done. A live session tears down its own in-flight work and sends the done +%% session-closed, so clients waiting for it (vim-fireplace) don't loop. A +%% live session tears down its own in-flight work and sends the reply %% itself - routing close through the async request path like any other %% session op keeps message flow one-directional (conn never blocks on a %% session that might be blocking on the conn). close(Req, State) -> case maps:get(<<"session">>, Req, undefined) of undefined -> - send_self(dialtone_msg:response(Req, #{<<"status">> => [done]}), State); + send_self(dialtone_msg:response(Req, #{<<"status">> => ['session-closed', done]}), + State); Id -> case dialtone_sessions:lookup(Id) of {ok, Pid} -> dialtone_session:request(Pid, Req, self()); error -> - send_self(dialtone_msg:response(Req, #{<<"status">> => [done]}), + send_self(dialtone_msg:response(Req, #{<<"status">> => ['session-closed', done]}), State) end end. diff --git a/dialtone/src/dialtone_session.erl b/dialtone/src/dialtone_session.erl index d4b508a..4f9719a 100644 --- a/dialtone/src/dialtone_session.erl +++ b/dialtone/src/dialtone_session.erl @@ -71,7 +71,15 @@ handle_call(get_bstate, _From, State) -> handle_cast({request, #{<<"op">> := <<"close">>} = Req, ReplyTo}, State) -> interrupt_all(State), - dialtone_msg:reply_done(ReplyTo, Req, #{}), + %% Unregister before replying: once a client sees session-closed, its + %% next request must get unknown-session instead of reaching this + %% process while it shuts down. + ok = case State#session.ephemeral of + true -> ok; + false -> dialtone_sessions:remove(State#session.id) + end, + %% vim-fireplace only forgets a session once it sees session-closed. + dialtone_msg:reply_done(ReplyTo, Req, ['session-closed'], #{}), {stop, normal, State#session{running = undefined, queue = queue:new()}}; handle_cast({request, Req, ReplyTo}, State) -> {noreply, handle_request(maps:get(<<"op">>, Req), Req, ReplyTo, State)}. diff --git a/dialtone/src/dialtone_sessions.erl b/dialtone/src/dialtone_sessions.erl index 1422bd4..3e5a994 100644 --- a/dialtone/src/dialtone_sessions.erl +++ b/dialtone/src/dialtone_sessions.erl @@ -5,7 +5,7 @@ -behaviour(gen_server). --export([start_link/0, new/2, lookup/1, list/0]). +-export([start_link/0, new/2, lookup/1, list/0, remove/1]). -export([init/1, handle_call/3, handle_cast/2, handle_info/2]). -define(TAB, ?MODULE). @@ -30,10 +30,21 @@ lookup(Id) -> list() -> [Id || {Id, _Pid} <- ets:tab2list(?TAB)]. +%% @doc Forget a session right away instead of when its process goes down, +%% so a client that's been told the session is closed can't reach it any +%% more. Called by the session itself while handling close. +remove(Id) -> + gen_server:call(?MODULE, {remove, Id}). + init([]) -> ?TAB = ets:new(?TAB, [named_table, protected, {read_concurrency, true}]), {ok, #{}}. +handle_call({remove, Id}, _From, Monitors) -> + %% The monitor stays: the DOWN that follows deletes the row again, which + %% is harmless. + true = ets:delete(?TAB, Id), + {reply, ok, Monitors}; handle_call({new, Backend, InitialBState}, _From, Monitors) -> Id = dialtone_uuid:v4(), case dialtone_session_sup:start_session(Id, Backend, InitialBState) of diff --git a/dialtone/test/dialtone_SUITE.erl b/dialtone/test/dialtone_SUITE.erl index 2e65817..3b227da 100644 --- a/dialtone/test/dialtone_SUITE.erl +++ b/dialtone/test/dialtone_SUITE.erl @@ -25,6 +25,7 @@ all() -> stdin_roundtrip, stdin_eof, ls_sessions_and_close, + eval_after_close, ephemeral_eval, clone_inherits_bindings, concurrent_sessions, @@ -203,7 +204,7 @@ ls_sessions_and_close(Config) -> {[CloseResp], Client3} = dialtone_client:request(Client2, #{<<"op">> => <<"close">>, <<"session">> => Session}), - ?assertEqual([<<"done">>], maps:get(<<"status">>, CloseResp)), + ?assertEqual([<<"session-closed">>, <<"done">>], maps:get(<<"status">>, CloseResp)), %% registry cleanup is async (DOWN); poll briefly ok = wait_until(fun() -> {[Ls2], _} = dialtone_client:request( @@ -215,7 +216,21 @@ ls_sessions_and_close(Config) -> {[CloseResp2], _} = dialtone_client:request(Client3, #{<<"op">> => <<"close">>, <<"session">> => Session}), - ?assertEqual([<<"done">>], maps:get(<<"status">>, CloseResp2)). + ?assertEqual([<<"session-closed">>, <<"done">>], maps:get(<<"status">>, CloseResp2)). + +%% Once a client has seen session-closed, the session has to be gone: a +%% request sent right after it gets unknown-session instead of hanging. +eval_after_close(Config) -> + lists:foreach( + fun(_) -> + {Session, Client} = clone(?config(client, Config)), + {[_], Client2} = dialtone_client:request(Client, #{<<"op">> => <<"close">>, + <<"session">> => Session}), + {Msgs, _} = dialtone_client:request(Client2, #{<<"op">> => <<"eval">>, + <<"session">> => Session, + <<"code">> => <<"ok.">>}), + ?assert(lists:member(<<"unknown-session">>, statuses_of(Msgs))) + end, lists:seq(1, 25)). ephemeral_eval(Config) -> {Msgs, _} = dialtone_client:request(?config(client, Config), @@ -306,7 +321,7 @@ close_mid_eval(Config) -> {DoomedMsgs, Client4} = dialtone_client:recv_until_done(Client3, <<"doomed">>, 5000), ?assert(lists:member(<<"interrupted">>, statuses_of(DoomedMsgs))), {CloseMsgs, _} = dialtone_client:recv_until_done(Client4, <<"closer">>, 5000), - ?assertEqual([<<"done">>], statuses_of(CloseMsgs)). + ?assertEqual([<<"done">>, <<"session-closed">>], statuses_of(CloseMsgs)). load_file_module(Config) -> {Session, Client} = clone(?config(client, Config)), From 856166f89e4c17d535026331fdfbdf1dc42b21f8 Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 1 Oct 2026 15:37:31 +0300 Subject: [PATCH 2/6] Echo the op in unknown-op replies Calva builds its "the server doesn't support this" message from the op in the reply, and nREPL includes it as well. All unknown-op replies now come from one helper, including those for optional ops a backend doesn't implement. --- dialtone/src/dialtone_conn.erl | 4 ++-- dialtone/src/dialtone_msg.erl | 9 ++++++++- dialtone/src/dialtone_session.erl | 2 +- dialtone/test/dialtone_SUITE.erl | 20 +++++++++++++++++++- dialtone/test/dialtone_minimal_backend.erl | 16 ++++++++++++++++ 5 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 dialtone/test/dialtone_minimal_backend.erl diff --git a/dialtone/src/dialtone_conn.erl b/dialtone/src/dialtone_conn.erl index db220ed..e6dfcd5 100644 --- a/dialtone/src/dialtone_conn.erl +++ b/dialtone/src/dialtone_conn.erl @@ -94,7 +94,7 @@ drain(Buffer, State) -> handle_message(Req, State) -> case maps:get(<<"op">>, Req, undefined) of - undefined -> send_self(dialtone_msg:response(Req, #{<<"status">> => ['unknown-op', done]}), State); + undefined -> send_self(dialtone_msg:unknown_op(Req), State); <<"describe">> -> describe(Req, State); <<"clone">> -> clone(Req, State); <<"ls-sessions">> -> ls_sessions(Req, State); @@ -103,7 +103,7 @@ handle_message(Req, State) -> Op =:= <<"interrupt">>; Op =:= <<"completions">>; Op =:= <<"lookup">> -> route_to_session(Req, State); _UnknownOp -> - send_self(dialtone_msg:response(Req, #{<<"status">> => ['unknown-op', done]}), State) + send_self(dialtone_msg:unknown_op(Req), State) end. describe(Req, #conn{backend = {BMod, _}} = State) -> diff --git a/dialtone/src/dialtone_msg.erl b/dialtone/src/dialtone_msg.erl index fad60fe..e4aea8e 100644 --- a/dialtone/src/dialtone_msg.erl +++ b/dialtone/src/dialtone_msg.erl @@ -11,13 +11,20 @@ %% design, and results of an eval whose client went away have nowhere to go. -module(dialtone_msg). --export([reply/3, reply_done/3, reply_done/4, send/2, response/2]). +-export([reply/3, reply_done/3, reply_done/4, send/2, response/2, unknown_op/1]). %% @doc Build a response map echoing id/session from the request. -spec response(map(), map()) -> map(). response(Req, Extra) -> maps:merge(maps:with([<<"id">>, <<"session">>], Req), Extra). +%% @doc The response to an op the server doesn't support. It echoes the op, +%% which Calva uses to tell the user what's missing. +-spec unknown_op(map()) -> map(). +unknown_op(Req) -> + response(Req, maps:merge(maps:with([<<"op">>], Req), + #{<<"status">> => ['unknown-op', done]})). + %% @doc Send an intermediate (non-terminal) response for a request. -spec reply(pid(), map(), map()) -> ok. reply(Conn, Req, Extra) -> diff --git a/dialtone/src/dialtone_session.erl b/dialtone/src/dialtone_session.erl index 4f9719a..035d647 100644 --- a/dialtone/src/dialtone_session.erl +++ b/dialtone/src/dialtone_session.erl @@ -166,7 +166,7 @@ ro_meta(Req) -> read_only(F, Args, Render, Req, ReplyTo, #session{backend = {BMod, _}} = State) -> case erlang:function_exported(BMod, F, length(Args) + 1) of false -> - dialtone_msg:reply_done(ReplyTo, Req, ['unknown-op'], #{}); + dialtone_msg:send(ReplyTo, dialtone_msg:unknown_op(Req)); true -> try apply(BMod, F, Args ++ [State#session.bstate]) of Result -> diff --git a/dialtone/test/dialtone_SUITE.erl b/dialtone/test/dialtone_SUITE.erl index 3b227da..cc51863 100644 --- a/dialtone/test/dialtone_SUITE.erl +++ b/dialtone/test/dialtone_SUITE.erl @@ -19,6 +19,7 @@ all() -> syntax_error, runtime_error, unknown_op, + unsupported_backend_op, missing_code, stdout_streaming, stdout_before_done, @@ -135,7 +136,24 @@ runtime_error(Config) -> unknown_op(Config) -> {[Resp], _} = dialtone_client:request(?config(client, Config), #{<<"op">> => <<"frobnicate">>}), - ?assertEqual([<<"unknown-op">>, <<"done">>], maps:get(<<"status">>, Resp)). + ?assertEqual([<<"unknown-op">>, <<"done">>], maps:get(<<"status">>, Resp)), + ?assertEqual(<<"frobnicate">>, maps:get(<<"op">>, Resp)). + +%% Ops a backend doesn't implement are answered by the session, and they +%% echo the op too. +unsupported_backend_op(_Config) -> + {ok, Server} = dialtone:start_server(#{port => 0, port_file => false, + backend => {dialtone_minimal_backend, #{}}}), + try + {Session, Client} = clone(dialtone_client:connect(dialtone:port(Server))), + {[Resp], _} = dialtone_client:request(Client, #{<<"op">> => <<"completions">>, + <<"session">> => Session, + <<"prefix">> => <<"li">>}), + ?assertEqual([<<"unknown-op">>, <<"done">>], maps:get(<<"status">>, Resp)), + ?assertEqual(<<"completions">>, maps:get(<<"op">>, Resp)) + after + ok = dialtone:stop_server(Server) + end. missing_code(Config) -> {Session, Client} = clone(?config(client, Config)), diff --git a/dialtone/test/dialtone_minimal_backend.erl b/dialtone/test/dialtone_minimal_backend.erl new file mode 100644 index 0000000..4c46785 --- /dev/null +++ b/dialtone/test/dialtone_minimal_backend.erl @@ -0,0 +1,16 @@ +%% A backend with only the required callbacks, for testing how dialtone +%% answers the optional ops it doesn't implement. +-module(dialtone_minimal_backend). + +-behaviour(dialtone_backend). + +-export([init/1, eval/3, version_info/0]). + +init(_Opts) -> + {ok, #{}}. + +eval(Code, _Meta, State) -> + {ok, #{value => Code}, State}. + +version_info() -> + #{}. From 61186fa7619f52e6ff71fda6cac880230eec23f8 Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 1 Oct 2026 15:37:40 +0300 Subject: [PATCH 3/6] Answer an eval without code with no-code Nothing gets evaluated in that case, so eval-error was misleading, and nREPL answers with no-code as well. A code field that isn't a string is still reported as a malformed request. --- dialtone/src/dialtone_session.erl | 7 ++++++- dialtone/test/dialtone_SUITE.erl | 12 +++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/dialtone/src/dialtone_session.erl b/dialtone/src/dialtone_session.erl index 035d647..c219140 100644 --- a/dialtone/src/dialtone_session.erl +++ b/dialtone/src/dialtone_session.erl @@ -123,6 +123,10 @@ handle_request(Op, Req, ReplyTo, State) case validate(Op, Req) of ok -> enqueue_or_run({Req, ReplyTo}, State); + no_code -> + %% Nothing was evaluated, so this isn't an eval-error. + dialtone_msg:reply_done(ReplyTo, Req, ['no-code'], #{}), + State; {error, Err} -> dialtone_msg:reply_done(ReplyTo, Req, ['eval-error'], #{<<"err">> => Err, <<"ex">> => <<"bad-request">>}), @@ -151,7 +155,8 @@ handle_request(<<"lookup">>, Req, ReplyTo, State) -> end, Req, ReplyTo, State). validate(<<"eval">>, #{<<"code">> := Code}) when is_binary(Code) -> ok; -validate(<<"eval">>, _) -> {error, <<"eval op requires a code field">>}; +validate(<<"eval">>, #{<<"code">> := _}) -> {error, <<"the code field must be a string">>}; +validate(<<"eval">>, _) -> no_code; validate(<<"load-file">>, #{<<"file">> := File}) when is_binary(File) -> ok; validate(<<"load-file">>, _) -> {error, <<"load-file op requires a file field">>}. diff --git a/dialtone/test/dialtone_SUITE.erl b/dialtone/test/dialtone_SUITE.erl index cc51863..ed531bf 100644 --- a/dialtone/test/dialtone_SUITE.erl +++ b/dialtone/test/dialtone_SUITE.erl @@ -157,9 +157,15 @@ unsupported_backend_op(_Config) -> missing_code(Config) -> {Session, Client} = clone(?config(client, Config)), - {Msgs, _} = dialtone_client:request(Client, #{<<"op">> => <<"eval">>, - <<"session">> => Session}), - ?assert(lists:member(<<"eval-error">>, statuses_of(Msgs))). + {Msgs, Client2} = dialtone_client:request(Client, #{<<"op">> => <<"eval">>, + <<"session">> => Session}), + ?assertEqual([<<"done">>, <<"no-code">>], statuses_of(Msgs)), + ?assertEqual([], [M || #{<<"err">> := _} = M <- Msgs]), + %% code that's there but isn't a string is a malformed request + {Msgs2, _} = dialtone_client:request(Client2, #{<<"op">> => <<"eval">>, + <<"session">> => Session, + <<"code">> => 42}), + ?assert(lists:member(<<"eval-error">>, statuses_of(Msgs2))). stdout_streaming(Config) -> {Session, Client} = clone(?config(client, Config)), From 1a96114ff370842c69b56b11e5762d8e476d04aa Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 1 Oct 2026 15:41:39 +0300 Subject: [PATCH 4/6] Stop temporary sessions that don't start an eval A request without a session gets a session of its own, which stops once its eval finishes. Requests that were answered right away (an eval without code, completions, lookup and the like) never got that far, so their sessions stayed around for good. --- dialtone/src/dialtone_session.erl | 9 ++++++++- dialtone/test/dialtone_SUITE.erl | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/dialtone/src/dialtone_session.erl b/dialtone/src/dialtone_session.erl index c219140..f533459 100644 --- a/dialtone/src/dialtone_session.erl +++ b/dialtone/src/dialtone_session.erl @@ -82,7 +82,14 @@ handle_cast({request, #{<<"op">> := <<"close">>} = Req, ReplyTo}, State) -> dialtone_msg:reply_done(ReplyTo, Req, ['session-closed'], #{}), {stop, normal, State#session{running = undefined, queue = queue:new()}}; handle_cast({request, Req, ReplyTo}, State) -> - {noreply, handle_request(maps:get(<<"op">>, Req), Req, ReplyTo, State)}. + case handle_request(maps:get(<<"op">>, Req), Req, ReplyTo, State) of + %% A throwaway session whose request was answered without starting + %% an eval (no-code, completions, ...) has nothing left to do. + #session{ephemeral = true, running = undefined} = State1 -> + {stop, normal, State1}; + State1 -> + {noreply, State1} + end. handle_info({eval_result, Ref, Result}, #session{running = #{ref := Ref, req := Req, reply_to := ReplyTo, diff --git a/dialtone/test/dialtone_SUITE.erl b/dialtone/test/dialtone_SUITE.erl index ed531bf..4d76974 100644 --- a/dialtone/test/dialtone_SUITE.erl +++ b/dialtone/test/dialtone_SUITE.erl @@ -28,6 +28,7 @@ all() -> ls_sessions_and_close, eval_after_close, ephemeral_eval, + ephemeral_sessions_stop, clone_inherits_bindings, concurrent_sessions, interrupt_running_eval, @@ -517,3 +518,21 @@ wait_until(Fun, N) -> true -> ok; false -> timer:sleep(20), wait_until(Fun, N - 1) end. + +%% Sessionless requests that don't start an eval mustn't leave their +%% throwaway session behind. +ephemeral_sessions_stop(Config) -> + Sessions = fun() -> + length([P || P <- processes(), + proc_lib:translate_initial_call(P) =:= + {dialtone_session, init, 1}]) + end, + Before = Sessions(), + lists:foreach( + fun(Req) -> + {_, _} = dialtone_client:request(?config(client, Config), Req) + end, + [#{<<"op">> => <<"eval">>}, + #{<<"op">> => <<"completions">>, <<"prefix">> => <<"li">>}, + #{<<"op">> => <<"lookup">>, <<"sym">> => <<"lists">>}]), + ok = wait_until(fun() -> Sessions() =< Before end). From 12d6fd52110668013bb9ab0f27910caeaa3675b5 Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 1 Oct 2026 15:39:47 +0300 Subject: [PATCH 5/6] Return a value for every Erlang form in an eval Evaluating "1. 2." used to report only the last value, so clients showed one result and silently dropped the rest. Now each form's value goes to the client as soon as the form is done, like the shell prints them. The values travel through the session's IO server, the same way output does, so they arrive in step with it, and an error in a later form no longer swallows the earlier values. Other backends can do the same through dialtone_backend:emit_value/1. load-file still reports only the last value, as nREPL does. --- dialtone/src/dialtone_backend.erl | 14 +++++++++ dialtone/src/dialtone_backend_erlang.erl | 36 ++++++++++++++++-------- dialtone/src/dialtone_io_server.erl | 17 +++++++++++ dialtone/src/dialtone_msg.erl | 15 +++++++++- dialtone/src/dialtone_session.erl | 12 +------- dialtone/test/dialtone_SUITE.erl | 26 +++++++++++++++-- 6 files changed, 93 insertions(+), 27 deletions(-) diff --git a/dialtone/src/dialtone_backend.erl b/dialtone/src/dialtone_backend.erl index ac5927e..8a278dc 100644 --- a/dialtone/src/dialtone_backend.erl +++ b/dialtone/src/dialtone_backend.erl @@ -9,6 +9,10 @@ %% resources they'd need to clean up. Output streaming needs no callback: %% evaluated code writes through its group leader, which dialtone owns. %% +%% A backend that evaluates several forms in one eval reports the value of +%% each form but the last with {@link emit_value/1} as it goes; the last one +%% is the `value' that eval/3 returns. +%% %% Optional callbacks gate op advertisement: a backend without `complete/3' %% simply yields a server that doesn't announce (or answer) `completions'. -module(dialtone_backend). @@ -23,6 +27,8 @@ %% "arglists-str", "file", "line"; values binary or integer. -type info() :: #{binary() => binary() | integer()}. +-export([emit_value/1]). + -export_type([state/0, eval_meta/0, ok_result/0, err_result/0, candidate/0, info/0]). @@ -56,3 +62,11 @@ -optional_callbacks([load_file/3, complete/3, lookup/3, format_exception/4, io_opts/0]). + +%% @doc Send a value to the client right away, ahead of the one eval/3 +%% returns. Only meaningful inside eval/3, where the group leader is the +%% session's IO server. +-spec emit_value(unicode:chardata()) -> ok. +emit_value(Value) -> + _ = io:request(group_leader(), {dialtone_value, Value}), + ok. diff --git a/dialtone/src/dialtone_backend_erlang.erl b/dialtone/src/dialtone_backend_erlang.erl index 20625b0..656bc01 100644 --- a/dialtone/src/dialtone_backend_erlang.erl +++ b/dialtone/src/dialtone_backend_erlang.erl @@ -3,7 +3,9 @@ %% %% Code is scanned/parsed/evaluated with the stdlib erl_scan/erl_parse/ %% erl_eval pipeline. Dot-terminated forms evaluate in sequence, threading -%% bindings; the last value is reported. A missing final dot is forgiven +%% bindings, and each form's value is reported as it's computed, like the +%% shell prints them (load-file reports only the last one). A missing final +%% dot is forgiven %% (editors send bare expressions like "1 + 2"). Evaluating module %% definitions is not supported by erl_eval - load-file (M3) compiles module %% sources via compile:forms instead. @@ -18,7 +20,11 @@ init(_Opts) -> %% so lookup can read their debug_info chunk (they exist only in memory). {ok, #{bindings => erl_eval:new_bindings(), beams => #{}}}. -eval(Code, Meta, #{bindings := Bindings} = State) -> +eval(Code, Meta, State) -> + eval(Code, Meta, State, true). + +%% EmitAll says whether forms before the last report their values too. +eval(Code, Meta, #{bindings := Bindings} = State, EmitAll) -> StartLoc = {maps:get(line, Meta, 1), maps:get(column, Meta, 1)}, Str = unicode:characters_to_list(Code), case erl_scan:string(Str, StartLoc) of @@ -29,7 +35,7 @@ eval(Code, Meta, #{bindings := Bindings} = State) -> {ok, Tokens, EndLoc} -> case parse_forms(ensure_dot(Tokens, EndLoc)) of {ok, ExprLists} -> - eval_forms(ExprLists, Bindings, State); + eval_forms(ExprLists, Bindings, State, EmitAll); {error, ErrResult} -> {error, ErrResult, State} end; @@ -49,7 +55,8 @@ load_file(Contents, Meta, State) -> Forms = split_dots(ensure_dot(Tokens, EndLoc)), case is_module_source(Forms) of true -> compile_module(Forms, Path, State); - false -> eval(Contents, #{file => Path}, State) + %% Like nREPL, load-file reports just the last form's value. + false -> eval(Contents, #{file => Path}, State, false) end; {error, {Loc, Mod, Desc}, _} -> {error, syntax_error(Loc, Mod, Desc), State} @@ -374,14 +381,19 @@ split_dots([Token | Rest], Current, Forms) -> %% Runtime errors raise out of erl_eval and are rendered by the worker's %% catch-all (dialtone_err) - by design, so users see real stacktraces. -eval_forms(ExprLists, Bindings0, State) -> - {Value, Bindings} = - lists:foldl(fun(Exprs, {_PrevValue, B}) -> - {value, V, B2} = erl_eval:exprs(Exprs, B), - {V, B2} - end, {ok, Bindings0}, ExprLists), - {ok, #{value => io_lib:format("~tp", [Value])}, - State#{bindings := Bindings}}. +eval_forms([Exprs], Bindings0, State, _EmitAll) -> + {value, Value, Bindings} = erl_eval:exprs(Exprs, Bindings0), + {ok, #{value => format_value(Value)}, State#{bindings := Bindings}}; +eval_forms([Exprs | Rest], Bindings0, State, EmitAll) -> + {value, Value, Bindings} = erl_eval:exprs(Exprs, Bindings0), + case EmitAll of + true -> dialtone_backend:emit_value(format_value(Value)); + false -> ok + end, + eval_forms(Rest, Bindings, State, EmitAll). + +format_value(Value) -> + io_lib:format("~tp", [Value]). syntax_error(Loc, Mod, Desc) -> Where = case Loc of diff --git a/dialtone/src/dialtone_io_server.erl b/dialtone/src/dialtone_io_server.erl index c60016b..21033e3 100644 --- a/dialtone/src/dialtone_io_server.erl +++ b/dialtone/src/dialtone_io_server.erl @@ -84,6 +84,18 @@ io_request(From, ReplyAs, {put_chars, Chars}, State) -> io_request(From, ReplyAs, {put_chars, latin1, Chars}, State); io_request(From, ReplyAs, {put_chars, M, F, A}, State) -> io_request(From, ReplyAs, {put_chars, latin1, M, F, A}, State); +%% A value from a form that isn't the last one in an eval, sent by the +%% backend (see dialtone_backend:emit_value/1). It goes the same way as +%% output, so it reaches the client in step with the code's output. +io_request(From, ReplyAs, {dialtone_value, Value}, #io{sink = Sink} = State) -> + Reply = case unicode:characters_to_binary(Value) of + Bin when is_binary(Bin) -> + emit_value(dialtone_msg:truncate_value(Bin), Sink); + _ -> + {error, {no_translation, unicode, utf8}} + end, + io_reply(From, ReplyAs, Reply), + State; io_request(From, ReplyAs, {get_until, Encoding, Prompt, M, F, A}, State) -> enqueue_read(From, ReplyAs, {until, Encoding, M, F, A, []}, Prompt, State); io_request(From, ReplyAs, {get_line, Encoding, Prompt}, State) -> @@ -137,6 +149,11 @@ do_put_chars(Encoding, Chars, #io{sink = Sink}) -> {error, {no_translation, Encoding, utf8}} end. +emit_value(_Bin, undefined) -> + ok; +emit_value(Bin, {Conn, Req}) -> + dialtone_msg:reply(Conn, Req, #{<<"value">> => Bin}). + emit_out(_Bin, undefined) -> ok; emit_out(Bin, {Conn, Req}) -> diff --git a/dialtone/src/dialtone_msg.erl b/dialtone/src/dialtone_msg.erl index e4aea8e..cb6fc41 100644 --- a/dialtone/src/dialtone_msg.erl +++ b/dialtone/src/dialtone_msg.erl @@ -11,7 +11,12 @@ %% design, and results of an eval whose client went away have nowhere to go. -module(dialtone_msg). --export([reply/3, reply_done/3, reply_done/4, send/2, response/2, unknown_op/1]). +-export([reply/3, reply_done/3, reply_done/4, send/2, response/2, unknown_op/1, + truncate_value/1]). + +%% Crude safety net against a multi-gigabyte value taking the editor down; +%% a proper client-negotiated print quota can replace it later. +-define(MAX_VALUE_BYTES, 8 * 1024 * 1024). %% @doc Build a response map echoing id/session from the request. -spec response(map(), map()) -> map(). @@ -25,6 +30,14 @@ unknown_op(Req) -> response(Req, maps:merge(maps:with([<<"op">>], Req), #{<<"status">> => ['unknown-op', done]})). +%% @doc Cut a rendered value down to a size clients can cope with. +-spec truncate_value(binary()) -> binary(). +truncate_value(Bin) when byte_size(Bin) > ?MAX_VALUE_BYTES -> + Head = binary:part(Bin, 0, ?MAX_VALUE_BYTES), + <>; +truncate_value(Bin) -> + Bin. + %% @doc Send an intermediate (non-terminal) response for a request. -spec reply(pid(), map(), map()) -> ok. reply(Conn, Req, Extra) -> diff --git a/dialtone/src/dialtone_session.erl b/dialtone/src/dialtone_session.erl index f533459..bd28366 100644 --- a/dialtone/src/dialtone_session.erl +++ b/dialtone/src/dialtone_session.erl @@ -17,10 +17,6 @@ -export([start_link/3, start_ephemeral/2, request/3, get_bstate/1]). -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2]). -%% Crude safety net against a multi-gigabyte value taking the editor down; -%% a proper client-negotiated print quota can replace it later. --define(MAX_VALUE_BYTES, 8 * 1024 * 1024). - -record(session, {id :: binary(), backend :: {module(), map()}, bstate :: term(), @@ -231,7 +227,7 @@ eval_meta(Req) -> deliver_result(Req, ReplyTo, Result, #session{bstate = OldBState}) -> case Result of {ok, OkMap, NewBState} -> - Value = truncate(unicode:characters_to_binary(maps:get(value, OkMap))), + Value = dialtone_msg:truncate_value(unicode:characters_to_binary(maps:get(value, OkMap))), Extra = case OkMap of #{ns := Ns} -> #{<<"ns">> => unicode:characters_to_binary(Ns)}; _ -> #{} @@ -251,12 +247,6 @@ deliver_result(Req, ReplyTo, Result, #session{bstate = OldBState}) -> OldBState end. -truncate(Bin) when byte_size(Bin) > ?MAX_VALUE_BYTES -> - Head = binary:part(Bin, 0, ?MAX_VALUE_BYTES), - <>; -truncate(Bin) -> - Bin. - next(#session{ephemeral = true, running = undefined} = State) -> %% One request served; this throwaway session is done. {stop, normal, State}; diff --git a/dialtone/test/dialtone_SUITE.erl b/dialtone/test/dialtone_SUITE.erl index 4d76974..537206f 100644 --- a/dialtone/test/dialtone_SUITE.erl +++ b/dialtone/test/dialtone_SUITE.erl @@ -113,8 +113,21 @@ bindings_persist(Config) -> multi_form_eval(Config) -> {Session, Client} = clone(?config(client, Config)), - {Msgs, _} = eval(Client, Session, <<"A = 1. B = A + 1. A + B.">>), - ?assertEqual(<<"3">>, value_of(Msgs)). + {Msgs, Client2} = eval(Client, Session, <<"A = 1. B = A + 1. A + B.">>), + %% one value per form, like the shell prints them + ?assertEqual([<<"1">>, <<"2">>, <<"3">>], [V || #{<<"value">> := V} <- Msgs]), + %% each value goes out as soon as its form is done, so it arrives in + %% step with the output + {Msgs2, Client3} = eval(Client2, Session, <<"io:format(\"a\"). io:format(\"b\").">>), + ?assertEqual([{out, <<"a">>}, {value, <<"ok">>}, {out, <<"b">>}, {value, <<"ok">>}], + [case M of + #{<<"out">> := O} -> {out, O}; + #{<<"value">> := V} -> {value, V} + end || M <- Msgs2, is_map_key(<<"out">>, M) orelse is_map_key(<<"value">>, M)]), + %% an error in a later form doesn't swallow the earlier values + {Msgs3, _} = eval(Client3, Session, <<"1. 1 div 0.">>), + ?assertEqual([<<"1">>], [V || #{<<"value">> := V} <- Msgs3]), + ?assert(lists:member(<<"eval-error">>, statuses_of(Msgs3))). syntax_error(Config) -> {Session, Client} = clone(?config(client, Config)), @@ -374,7 +387,14 @@ load_file_expressions(Config) -> Client, #{<<"op">> => <<"load-file">>, <<"session">> => Session, <<"file">> => <<"A = 20, B = 22, A + B.">>}), - ?assertEqual(<<"42">>, value_of(Msgs)). + ?assertEqual(<<"42">>, value_of(Msgs)), + %% several forms still report only the last value, like nREPL + {Session2, Client2} = clone(?config(client, Config)), + {Msgs2, _} = dialtone_client:request( + Client2, #{<<"op">> => <<"load-file">>, + <<"session">> => Session2, + <<"file">> => <<"X = 1. Y = 2. X + Y.">>}), + ?assertEqual(<<"3">>, value_of(Msgs2)). load_file_compile_error(Config) -> {Session, Client} = clone(?config(client, Config)), From 86dcdb7b1ecb65d58817c10756c2d723cd608675 Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 1 Oct 2026 15:40:01 +0300 Subject: [PATCH 6/6] Don't fail load-file on a file without any code A file with nothing but comments (or nothing at all) crashed while looking for a module attribute. It's now treated like an empty eval. --- dialtone/src/dialtone_backend_erlang.erl | 3 +++ dialtone/test/dialtone_SUITE.erl | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dialtone/src/dialtone_backend_erlang.erl b/dialtone/src/dialtone_backend_erlang.erl index 656bc01..e4bf6c7 100644 --- a/dialtone/src/dialtone_backend_erlang.erl +++ b/dialtone/src/dialtone_backend_erlang.erl @@ -51,6 +51,9 @@ load_file(Contents, Meta, State) -> Path = maps:get(path, Meta, maps:get(name, Meta, <<"nrepl-load-file">>)), Str = unicode:characters_to_list(Contents), case erl_scan:string(Str) of + {ok, [], _EndLoc} -> + %% Nothing but whitespace and comments: same as an empty eval. + eval(Contents, #{file => Path}, State, false); {ok, Tokens, EndLoc} -> Forms = split_dots(ensure_dot(Tokens, EndLoc)), case is_module_source(Forms) of diff --git a/dialtone/test/dialtone_SUITE.erl b/dialtone/test/dialtone_SUITE.erl index 537206f..a59c34f 100644 --- a/dialtone/test/dialtone_SUITE.erl +++ b/dialtone/test/dialtone_SUITE.erl @@ -394,7 +394,14 @@ load_file_expressions(Config) -> Client2, #{<<"op">> => <<"load-file">>, <<"session">> => Session2, <<"file">> => <<"X = 1. Y = 2. X + Y.">>}), - ?assertEqual(<<"3">>, value_of(Msgs2)). + ?assertEqual(<<"3">>, value_of(Msgs2)), + %% a file with nothing but comments in it isn't an error + {Session3, Client3} = clone(?config(client, Config)), + {Msgs3, _} = dialtone_client:request( + Client3, #{<<"op">> => <<"load-file">>, + <<"session">> => Session3, + <<"file">> => <<"%% just a comment\n">>}), + ?assertNot(lists:member(<<"eval-error">>, statuses_of(Msgs3))). load_file_compile_error(Config) -> {Session, Client} = clone(?config(client, Config)),