Skip to content

Commit

Permalink
List module definitions in the sections side panel (#2760)
Browse files Browse the repository at this point in the history
  • Loading branch information
aleDsz authored Aug 23, 2024
1 parent eca2dc1 commit ad0db18
Show file tree
Hide file tree
Showing 15 changed files with 215 additions and 46 deletions.
14 changes: 13 additions & 1 deletion assets/js/hooks/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ const Session = {
}
} else if (keyBuffer.tryMatch(["e", "s"])) {
this.queueFocusedSectionEvaluation();
} else if (keyBuffer.tryMatch(["s", "s"])) {
} else if (keyBuffer.tryMatch(["s", "o"])) {
this.toggleSectionsList();
} else if (keyBuffer.tryMatch(["s", "e"])) {
this.toggleSecretsList();
Expand Down Expand Up @@ -579,11 +579,23 @@ const Session = {
*/
handleSectionsListClick(event) {
const sectionButton = event.target.closest(`[data-el-sections-list-item]`);

if (sectionButton) {
const sectionId = sectionButton.getAttribute("data-section-id");
const section = this.getSectionById(sectionId);
section.scrollIntoView({ behavior: "instant", block: "start" });
}

const sectionDefinitionButton = event.target.closest(
`[data-el-sections-list-definition-item]`,
);

if (sectionDefinitionButton) {
const file = sectionDefinitionButton.getAttribute("data-file");
const line = sectionDefinitionButton.getAttribute("data-line");

this.jumpToLine(file, line);
}
},

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/livebook/intellisense.ex
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ defmodule Livebook.Intellisense do
path = Path.join(context.ebin_path, "#{module}.beam")

with true <- File.exists?(path),
{:ok, line} <- Docs.locate_definition(path, identifier) do
{:ok, line} <- Docs.locate_definition(String.to_charlist(path), identifier) do
file = module.module_info(:compile)[:source]
%{file: to_string(file), line: line}
else
Expand Down
4 changes: 1 addition & 3 deletions lib/livebook/intellisense/docs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ defmodule Livebook.Intellisense.Docs do
The function returns the line where the identifier is located.
"""
@spec locate_definition(String.t(), definition()) :: {:ok, pos_integer()} | :error
@spec locate_definition(list() | binary(), definition()) :: {:ok, pos_integer()} | :error
def locate_definition(path, identifier)

def locate_definition(path, {:module, module}) do
Expand Down Expand Up @@ -221,8 +221,6 @@ defmodule Livebook.Intellisense.Docs do
end

defp beam_lib_chunks(path, key) do
path = String.to_charlist(path)

case :beam_lib.chunks(path, [key]) do
{:ok, {_, [{^key, value}]}} -> {:ok, value}
_ -> :error
Expand Down
5 changes: 4 additions & 1 deletion lib/livebook/runtime.ex
Original file line number Diff line number Diff line change
Expand Up @@ -470,12 +470,15 @@ defprotocol Livebook.Runtime do
dependencies between evaluations and avoids unnecessary reevaluations.
"""
@type evaluation_response_metadata :: %{
interrupted: boolean(),
errored: boolean(),
evaluation_time_ms: non_neg_integer(),
code_markers: list(code_marker()),
memory_usage: runtime_memory(),
identifiers_used: list(identifier :: term()) | :unknown,
identifiers_defined: %{(identifier :: term()) => version :: term()}
identifiers_defined: %{(identifier :: term()) => version :: term()},
identifier_definitions:
list(%{label: String.t(), file: String.t(), line: pos_integer()})
}

@typedoc """
Expand Down
29 changes: 22 additions & 7 deletions lib/livebook/runtime/evaluator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ defmodule Livebook.Runtime.Evaluator do

%{tracer_info: tracer_info} = Evaluator.IOProxy.after_evaluation(state.io_proxy)

{new_context, result, identifiers_used, identifiers_defined} =
{new_context, result, identifiers_used, identifiers_defined, identifier_definitions} =
case eval_result do
{:ok, value, binding, env} ->
context_id = random_long_id()
Expand All @@ -454,8 +454,10 @@ defmodule Livebook.Runtime.Evaluator do
{identifiers_used, identifiers_defined} =
identifier_dependencies(new_context, tracer_info, context)

identifier_definitions = definitions(new_context, tracer_info)

result = {:ok, value}
{new_context, result, identifiers_used, identifiers_defined}
{new_context, result, identifiers_used, identifiers_defined, identifier_definitions}

{:error, kind, error, stacktrace} ->
for {module, _} <- tracer_info.modules_defined do
Expand All @@ -465,17 +467,17 @@ defmodule Livebook.Runtime.Evaluator do
result = {:error, kind, error, stacktrace}
identifiers_used = :unknown
identifiers_defined = %{}
identifier_definitions = []
# Empty context
new_context = initial_context()
{new_context, result, identifiers_used, identifiers_defined}
{new_context, result, identifiers_used, identifiers_defined, identifier_definitions}
end

if ebin_path() do
Livebook.Runtime.Evaluator.Doctests.run(new_context.env.context_modules, code)
end

state = put_context(state, ref, new_context)

output = Evaluator.Formatter.format_result(result, language)

metadata = %{
Expand All @@ -485,7 +487,8 @@ defmodule Livebook.Runtime.Evaluator do
memory_usage: memory(),
code_markers: code_markers,
identifiers_used: identifiers_used,
identifiers_defined: identifiers_defined
identifiers_defined: identifiers_defined,
identifier_definitions: identifier_definitions
}

send(state.send_to, {:runtime_evaluation_response, ref, output, metadata})
Expand Down Expand Up @@ -890,7 +893,7 @@ defmodule Livebook.Runtime.Evaluator do
into: identifiers_used

identifiers_defined =
for {module, _vars} <- tracer_info.modules_defined,
for {module, _line_vars} <- tracer_info.modules_defined,
version = module.__info__(:md5),
do: {{:module, module}, version},
into: identifiers_defined
Expand Down Expand Up @@ -968,7 +971,7 @@ defmodule Livebook.Runtime.Evaluator do
# Note that :prune_binding removes variables used by modules
# (unless used outside), so we get those from the tracer
module_used_vars =
for {_module, vars} <- tracer_info.modules_defined,
for {_module, {_line, vars}} <- tracer_info.modules_defined,
var <- vars,
into: MapSet.new(),
do: var
Expand Down Expand Up @@ -1038,4 +1041,16 @@ defmodule Livebook.Runtime.Evaluator do
defp ebin_path() do
Process.get(@ebin_path_key)
end

defp definitions(context, tracer_info) do
for {module, {line, _vars}} <- tracer_info.modules_defined,
do: %{label: module_name(module), file: context.env.file, line: line}
end

defp module_name(module) do
case Atom.to_string(module) do
"Elixir." <> name -> name
name -> name
end
end
end
2 changes: 1 addition & 1 deletion lib/livebook/runtime/evaluator/io_proxy.ex
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ defmodule Livebook.Runtime.Evaluator.IOProxy do
state = update_in(state.tracer_info, &Evaluator.Tracer.apply_updates(&1, updates))

modules_defined =
for {:module_defined, module, _vars} <- updates,
for {:module_defined, module, _vars, _line} <- updates,
into: state.modules_defined,
do: module

Expand Down
6 changes: 3 additions & 3 deletions lib/livebook/runtime/evaluator/tracer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ defmodule Livebook.Runtime.Evaluator.Tracer do
module = env.module
vars = Map.keys(env.versioned_vars)
Evaluator.write_module!(module, bytecode)
[{:module_defined, module, vars}, {:alias_used, module}]
[{:module_defined, module, vars, env.line}, {:alias_used, module}]

_ ->
[]
Expand All @@ -112,8 +112,8 @@ defmodule Livebook.Runtime.Evaluator.Tracer do
update_in(info.modules_used, &MapSet.put(&1, module))
end

defp apply_update(info, {:module_defined, module, vars}) do
put_in(info.modules_defined[module], vars)
defp apply_update(info, {:module_defined, module, vars, line}) do
put_in(info.modules_defined[module], {line, vars})
end

defp apply_update(info, {:alias_used, alias}) do
Expand Down
6 changes: 5 additions & 1 deletion lib/livebook/session/data.ex
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ defmodule Livebook.Session.Data do
new_bound_to_inputs: %{input_id() => input_hash()},
identifiers_used: list(identifier :: term()) | :unknown,
identifiers_defined: %{(identifier :: term()) => version :: term()},
identifier_definitions:
list(%{label: String.t(), file: String.t(), line: pos_integer()}),
data: t()
}

Expand Down Expand Up @@ -1401,7 +1403,8 @@ defmodule Livebook.Session.Data do
identifiers_defined: metadata.identifiers_defined,
bound_to_inputs: eval_info.new_bound_to_inputs,
evaluation_end: DateTime.utc_now(),
code_markers: metadata.code_markers
code_markers: metadata.code_markers,
identifier_definitions: metadata.identifier_definitions
}
end)
|> update_cell_evaluation_snapshot(cell, section)
Expand Down Expand Up @@ -2380,6 +2383,7 @@ defmodule Livebook.Session.Data do
data: nil,
code_markers: [],
doctest_reports: %{},
identifier_definitions: [],
reevaluates_automatically: false
}
end
Expand Down
11 changes: 10 additions & 1 deletion lib/livebook_web/live/session_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,8 @@ defmodule LivebookWeb.SessionLive do
id: section.id,
name: section.name,
parent: parent_section_view(section.parent_id, data),
status: cells_status(section.cells, data)
status: cells_status(section.cells, data),
identifier_definitions: cells_identifier_definitions(section.cells, data)
}
end,
clients:
Expand Down Expand Up @@ -1853,6 +1854,14 @@ defmodule LivebookWeb.SessionLive do
end
end

defp cells_identifier_definitions(cells, data) do
for %Cell.Code{} = cell <- cells,
Cell.evaluable?(cell),
info = data.cell_infos[cell.id].eval,
definition <- info.identifier_definitions,
do: definition
end

defp global_status(data) do
cells =
data.notebook
Expand Down
65 changes: 42 additions & 23 deletions lib/livebook_web/live/session_live/render.ex
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,8 @@ defmodule LivebookWeb.SessionLive.Render do
<%!-- Local functionality --%>
<.button_item
icon="booklet-fill"
label="Sections (ss)"
icon="node-tree"
label="Outline (so)"
button_attrs={["data-el-sections-list-toggle": true]}
/>
Expand Down Expand Up @@ -419,8 +419,8 @@ defmodule LivebookWeb.SessionLive.Render do
class="flex flex-col h-full w-full max-w-xs absolute z-30 top-0 left-[64px] overflow-y-auto shadow-xl md:static md:shadow-none bg-gray-50 border-r border-gray-100 px-6 pt-16 md:py-8"
data-el-side-panel
>
<div class="flex grow" data-el-sections-list>
<.sections_list data_view={@data_view} />
<div data-el-sections-list>
<.outline_list data_view={@data_view} />
</div>
<div data-el-clients-list>
<.clients_list data_view={@data_view} client_id={@client_id} />
Expand Down Expand Up @@ -497,25 +497,26 @@ defmodule LivebookWeb.SessionLive.Render do
"""
end

defp sections_list(assigns) do
defp outline_list(assigns) do
~H"""
<div class="flex flex-col grow">
<h3 class="uppercase text-sm font-semibold text-gray-500">
Sections
Outline
</h3>
<div class="flex flex-col mt-4 space-y-4">
<div :for={section_item <- @data_view.sections_items} class="flex items-center">
<button
class="grow flex items-center text-gray-500 hover:text-gray-900 text-left"
data-el-sections-list-item
data-section-id={section_item.id}
>
<span class="flex items-center space-x-1">
<div :for={section_item <- @data_view.sections_items} class="flex flex-col">
<div class="flex justify-between items-center">
<button
class="grow flex items-center gap-1 text-gray-500 hover:text-gray-900 text-left"
data-el-sections-list-item
data-section-id={section_item.id}
>
<.remix_icon icon="font-size" class="text-lg font-normal leading-none" />
<span><%= section_item.name %></span>
<%!--
Note: the container has overflow-y auto, so we cannot set overflow-x visible,
consequently we show the tooltip wrapped to a fixed number of characters
--%>
Note: the container has overflow-y auto, so we cannot set overflow-x visible,
consequently we show the tooltip wrapped to a fixed number of characters
--%>
<span
:if={section_item.parent}
{branching_tooltip_attrs(section_item.name, section_item.parent.name)}
Expand All @@ -525,12 +526,30 @@ defmodule LivebookWeb.SessionLive.Render do
class="text-lg font-normal leading-none flip-horizontally"
/>
</span>
</span>
</button>
<.section_status
status={elem(section_item.status, 0)}
cell_id={elem(section_item.status, 1)}
/>
</button>
<.section_status
status={elem(section_item.status, 0)}
cell_id={elem(section_item.status, 1)}
/>
</div>
<ul :if={section_item.identifier_definitions != []} class="mt-2 ml-5 list-none items-center">
<li :for={definition <- section_item.identifier_definitions}>
<button
class="flex items-center max-w-full text-gray-500 hover:text-gray-900 text-sm gap-1"
data-el-sections-list-definition-item
data-file={definition.file}
data-line={definition.line}
title={definition.label}
>
<.remix_icon icon="braces-line" class="font-normal" />
<span class="font-mono truncate">
<%= definition.label %>
</span>
</button>
</li>
</ul>
</div>
</div>
<button
Expand Down Expand Up @@ -558,7 +577,7 @@ defmodule LivebookWeb.SessionLive.Render do
wrapped_name = Livebook.Utils.wrap_line("”" <> parent_name <> "”", 16)
label = "Branches from\n#{wrapped_name}"

[class: "tooltip #{direction}", data_tooltip: label]
[class: "tooltip #{direction}", "data-tooltip": label]
end

defp clients_list(assigns) do
Expand Down
Loading

0 comments on commit ad0db18

Please sign in to comment.