Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implements the Go to Definition keyboard shortcut #2741

Merged
merged 20 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assets/js/hooks/cell.js
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ const Cell = {

scrollIntoView(element, {
scrollMode: "if-needed",
behavior: "smooth",
behavior: "instant",
block: "center",
});
},
Expand Down
38 changes: 37 additions & 1 deletion assets/js/hooks/cell_editor/live_editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import { settingsStore } from "../../lib/settings";
import Delta from "../../lib/delta";
import Markdown from "../../lib/markdown";
import { readOnlyHint } from "./live_editor/codemirror/read_only_hint";
import { wait } from "../../lib/utils";
import { isMacOS, wait } from "../../lib/utils";
import Emitter from "../../lib/emitter";
import CollabClient from "./live_editor/collab_client";
import { languages } from "./live_editor/codemirror/languages";
Expand Down Expand Up @@ -363,14 +363,27 @@ export default class LiveEditor {
settings.editor_mode === "emacs" ? [emacs()] : [],
language ? language.support : [],
EditorView.domEventHandlers({
click: this.handleEditorClick.bind(this),
keydown: this.handleEditorKeydown.bind(this),
blur: this.handleEditorBlur.bind(this),
focus: this.handleEditorFocus.bind(this),
}),
EditorView.clickAddsSelectionRange.of((event) => event.altKey),
],
});
}

/** @private */
handleEditorClick(event) {
const cmd = isMacOS() ? event.metaKey : event.ctrlKey;

if (cmd) {
this.jumpToDefinition(this.view);
}

return false;
}

/** @private */
handleEditorKeydown(event) {
// We dispatch escape event, but only if it is not consumed by any
Expand Down Expand Up @@ -541,6 +554,29 @@ export default class LiveEditor {
.catch(() => null);
}

/** @private */
jumpToDefinition(view) {
const pos = view.state.selection.main.head;
const line = view.state.doc.lineAt(pos);
const lineLength = line.to - line.from;
const text = line.text;

const column = pos - line.from;
if (column < 1 || column > lineLength) return null;

return this.connection
.intellisenseRequest("definition", { line: text, column })
.then((response) => {
globalPubsub.broadcast("jump_to_editor", {
line: response.line,
file: response.file,
});

return true;
})
.catch(() => false);
}

/** @private */
signatureSource({ state, pos }) {
const textUntilCursor = this.getSignatureHint(state, pos);
Expand Down
23 changes: 18 additions & 5 deletions assets/js/hooks/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ const Session = {
(event) => this.toggleCollapseAllSections(),
);

this.subscriptions = [
globalPubsub.subscribe("jump_to_editor", ({ line, file }) =>
this.jumpToLine(file, line),
),
];

this.initializeDragAndDrop();

window.addEventListener(
Expand Down Expand Up @@ -270,6 +276,7 @@ const Session = {
leaveChannel();
}

this.subscriptions.forEach((subscription) => subscription.destroy());
this.store.destroy();
},

Expand Down Expand Up @@ -546,12 +553,9 @@ const Session = {
const search = event.target.hash.replace("#go-to-definition", "");
const params = new URLSearchParams(search);
const line = parseInt(params.get("line"), 10);
const [_filename, cellId] = params.get("file").split("#cell:");
const file = params.get("file");

this.setFocusedEl(cellId);
this.setInsertMode(true);

globalPubsub.broadcast(`cells:${cellId}`, { type: "jump_to_line", line });
this.jumpToLine(file, line);

event.preventDefault();
}
Expand Down Expand Up @@ -1442,6 +1446,15 @@ const Session = {
getElement(name) {
return this.el.querySelector(`[data-el-${name}]`);
},

jumpToLine(file, line) {
const [_filename, cellId] = file.split("#cell:");

this.setFocusedEl(cellId, { scroll: false, focusElement: true });
jonatanklosko marked this conversation as resolved.
Show resolved Hide resolved
this.setInsertMode(true);

globalPubsub.broadcast(`cells:${cellId}`, { type: "jump_to_line", line });
},
};

export default Session;
76 changes: 58 additions & 18 deletions lib/livebook/intellisense.ex
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ defmodule Livebook.Intellisense do
get_details(line, column, context, node)
end

def handle_request({:definition, line, column}, context, node) do
get_definitions(line, column, context, node)
end

def handle_request({:signature, hint}, context, node) do
get_signature_items(hint, context, node)
end
Expand Down Expand Up @@ -452,7 +456,7 @@ defmodule Livebook.Intellisense do
) do
join_with_divider([
code(inspect(module)),
format_definition_link(module, context),
format_definition_link(module, context, {:module, module}),
format_docs_link(module),
format_documentation(documentation, :all)
])
Expand Down Expand Up @@ -539,23 +543,9 @@ defmodule Livebook.Intellisense do
"""
end

defp format_definition_link(module, context, function_or_type \\ nil) do
if context.ebin_path do
path = Path.join(context.ebin_path, "#{module}.beam")

identifier =
if function_or_type,
do: function_or_type,
else: {:module, module}

with true <- File.exists?(path),
{:ok, line} <- Docs.locate_definition(path, identifier) do
file = module.module_info(:compile)[:source]
query_string = URI.encode_query(%{file: to_string(file), line: line})
"[Go to definition](#go-to-definition?#{query_string})"
else
_otherwise -> nil
end
defp format_definition_link(module, context, identifier) do
if query = get_definition_location(module, context, identifier) do
"[Go to definition](#go-to-definition?#{URI.encode_query(query)})"
end
end

Expand Down Expand Up @@ -710,6 +700,56 @@ defmodule Livebook.Intellisense do
raise "unknown documentation format #{inspect(format)}"
end

@doc """
Returns the identifier definition located in `column` in `line`.
"""
@spec get_definitions(String.t(), pos_integer(), context(), node()) ::
Runtime.definition_response() | nil
def get_definitions(line, column, context, node) do
case IdentifierMatcher.locate_identifier(line, column, context, node) do
%{matches: []} ->
nil

%{matches: matches, range: range} ->
matches
|> Enum.sort_by(& &1[:arity], :asc)
|> Enum.flat_map(&List.wrap(get_definition_location(&1, context)))
|> case do
[%{file: file, line: line} | _] -> %{range: range, file: file, line: line}
_ -> nil
end
end
end

defp get_definition_location(%{kind: :module, module: module}, context) do
get_definition_location(module, context, {:module, module})
end

defp get_definition_location(
%{kind: :function, module: module, name: name, arity: arity},
context
) do
get_definition_location(module, context, {:function, name, arity})
end

defp get_definition_location(%{kind: :type, module: module, name: name, arity: arity}, context) do
get_definition_location(module, context, {:type, name, arity})
end

defp get_definition_location(module, context, identifier) do
if context.ebin_path do
path = Path.join(context.ebin_path, "#{module}.beam")

with true <- File.exists?(path),
{:ok, line} <- Docs.locate_definition(path, identifier) do
file = module.module_info(:compile)[:source]
%{file: to_string(file), line: line}
else
_otherwise -> nil
end
end
end

# Erlang HTML AST
# See https://erlang.org/doc/apps/erl_docgen/doc_storage.html#erlang-documentation-format

Expand Down
28 changes: 22 additions & 6 deletions lib/livebook/intellisense/identifier_matcher.ex
Original file line number Diff line number Diff line change
Expand Up @@ -627,19 +627,35 @@ defmodule Livebook.Intellisense.IdentifierMatcher do
end

defp get_matching_modules(hint, ctx) do
ctx.node
ctx
|> get_modules()
|> Enum.filter(&ctx.matcher.(Atom.to_string(&1), hint))
|> Enum.uniq()
end

defp get_modules(node) do
modules = cached_all_loaded(node)

defp get_modules(%{node: node} = ctx) do
# On interactive mode, we load modules from the application
# and then the ones from runtime. For a remote node, ideally
# we would get the applications one, but there is no cheap
# way to do such, so we get :code.all_loaded and cache it
# instead (which includes all modules anyway on embedded mode).
if node == node() and :code.get_mode() == :interactive do
modules ++ get_modules_from_applications()
runtime_modules(ctx.intellisense_context.ebin_path) ++ get_modules_from_applications()
else
modules
cached_all_loaded(node)
end
end

defp runtime_modules(path) do
with true <- is_binary(path),
{:ok, beams} <- File.ls(path) do
for beam <- beams, String.ends_with?(beam, ".beam") do
beam
|> binary_slice(0..-6//1)
|> String.to_atom()
end
else
_ -> []
end
end

Expand Down
17 changes: 17 additions & 0 deletions lib/livebook/runtime.ex
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ defprotocol Livebook.Runtime do
@type intellisense_request ::
completion_request()
| details_request()
| definition_request()
| signature_request()
| format_request()

Expand All @@ -516,6 +517,7 @@ defprotocol Livebook.Runtime do
nil
| completion_response()
| details_response()
| definition_response()
| signature_response()
| format_response()

Expand Down Expand Up @@ -553,6 +555,21 @@ defprotocol Livebook.Runtime do
contents: list(String.t())
}

@typedoc """
Looks up more the definition about an identifier found in `column` in
`line`.
"""
@type definition_request :: {:definition, line :: String.t(), column :: pos_integer()}

@type definition_response :: %{
range: %{
from: non_neg_integer(),
to: non_neg_integer()
},
line: pos_integer(),
file: String.t()
}

@typedoc """
Looks up a list of function signatures matching the given hint.

Expand Down
4 changes: 4 additions & 0 deletions lib/livebook_web/live/session_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,10 @@ defmodule LivebookWeb.SessionLive do
column = Text.JS.js_column_to_elixir(column, line)
{:details, line, column}

%{"type" => "definition", "line" => line, "column" => column} ->
column = Text.JS.js_column_to_elixir(column, line)
{:definition, line, column}

%{"type" => "signature", "hint" => hint} ->
{:signature, hint}

Expand Down
Loading
Loading