-
Notifications
You must be signed in to change notification settings - Fork 157
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
feat: go-to-definition with cmd+click #1425
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
/* Copyright 2024 Marimo. All rights reserved. */ | ||
import { RefObject } from "react"; | ||
import type { RefObject } from "react"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Type import for RefObject Using |
||
import { Logger } from "../../utils/Logger"; | ||
import { CellId, HTMLCellId } from "./ids"; | ||
import { CellHandle } from "@/components/editor/Cell"; | ||
import { CellConfig } from "./types"; | ||
import { goToDefinition } from "../codemirror/find-replace/search-highlight"; | ||
import { type CellId, HTMLCellId } from "./ids"; | ||
import type { CellHandle } from "@/components/editor/Cell"; | ||
import type { CellConfig } from "./types"; | ||
import { goToVariableDefinition } from "../codemirror/go-to-definition/commands"; | ||
|
||
export function focusAndScrollCellIntoView({ | ||
cellId, | ||
|
@@ -57,7 +57,7 @@ export function focusAndScrollCellIntoView({ | |
}, | ||
}); | ||
} else if (variableName) { | ||
goToDefinition(editor, variableName); | ||
goToVariableDefinition(editor, variableName); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,15 +4,14 @@ import { RangeSetBuilder, StateEffect, StateField } from "@codemirror/state"; | |
import { | ||
Decoration, | ||
ViewPlugin, | ||
DecorationSet, | ||
type DecorationSet, | ||
EditorView, | ||
ViewUpdate, | ||
type ViewUpdate, | ||
} from "@codemirror/view"; | ||
import { QueryType, asQueryCreator } from "./query"; | ||
import { type QueryType, asQueryCreator } from "./query"; | ||
import { store } from "@/core/state/jotai"; | ||
import { findReplaceAtom } from "./state"; | ||
import { getAllEditorViews } from "@/core/cells/cells"; | ||
import { syntaxTree } from "@codemirror/language"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Unused import removed The removal of the |
||
|
||
const setSearchQuery = StateEffect.define<SearchQuery>(); | ||
|
||
|
@@ -147,57 +146,3 @@ export const highlightTheme = EditorView.baseTheme({ | |
backgroundColor: "#6199ff88 !important", | ||
}, | ||
}); | ||
|
||
/** | ||
* This function will select the first occurrence of the given variable name. | ||
*/ | ||
export function goToDefinition( | ||
view: EditorView, | ||
variableName: string, | ||
): boolean { | ||
const state = view.state; | ||
const tree = syntaxTree(state); | ||
|
||
let found = false; | ||
let from = 0; | ||
|
||
tree.iterate({ | ||
enter: (node) => { | ||
if (found) { | ||
return false; | ||
} // Stop traversal if found | ||
|
||
// Check if the node is an identifier and matches the variable name | ||
if ( | ||
node.name === "VariableName" && | ||
state.doc.sliceString(node.from, node.to) === variableName | ||
) { | ||
from = node.from; | ||
found = true; | ||
return false; // Stop traversal | ||
} | ||
|
||
// Skip comments and strings | ||
if (node.name === "Comment" || node.name === "String") { | ||
return false; | ||
} | ||
}, | ||
}); | ||
|
||
if (found) { | ||
view.focus(); | ||
view.dispatch({ | ||
selection: { | ||
anchor: from, | ||
head: from, | ||
}, | ||
// Unfortunately, EditorView.scrollIntoView does | ||
// not support smooth scrolling. | ||
effects: EditorView.scrollIntoView(from, { | ||
y: "center", | ||
}), | ||
}); | ||
return true; | ||
} | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Refactored splitCell function
The refactoring of the
splitCell
function to usesplitEditor
is a good improvement. However, ensure thatsplitEditor
handles all edge cases correctly, especially with different cursor positions and document states.