Skip to content

feat: add change selections command #1734

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

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

Conversation

Myriad-Dreamin
Copy link
Owner

Adding a command to track user selections in the editor, then we can use the focusing_selection stored in the global state:

/// The client focusing selection ranges.
///
/// To reduce complexity, we only record the primary selection range, while
/// there can be multiple ranges selected by the client at the same time.
/// - Example: In VS Code, you can create multiple cursors by `Ctrl + D`.
///
/// To reduce complexity, A further question that we haven't covered
/// currently is that: The client may select ranges in multiple files at
/// the same time, but we only preasume that all of the selections are
/// in the [`Self::focusing`] file.
/// - Example: In VS Code, you can pick multiple ranges in multiple files by
/// the text search command.
pub focusing_selection: Option<LspRange>,

This is unfriendly for the editors that cannot make client-side extensions.

}),
);
context.subscriptions.push(
vscode.workspace.onDidOpenTextDocument((doc: vscode.TextDocument) => {
if (doc.isUntitled && window.activeTextEditor?.document === doc) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the doc.isUntitled is a mistake, you probably meant !doc.isUntitled, because, just before on line 166 you return if the document is untitled which makes sense because you don't want to consider it.

Suggested change
if (doc.isUntitled && window.activeTextEditor?.document === doc) {
if (!doc.isUntitled && window.activeTextEditor?.document === doc) {

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I added comments to the code. I checked it and think of I'm not doing a bad refactor. Please check the comments:

// Watches the active editor that owning a titled `document` (`!isUntitled`).
//
// todo: plaintext detection
window.onDidChangeActiveTextEditor((editor: TextEditor | undefined) => {

// Watches the active editor that owning an untitled `document` (`isUntitled`).
// `onDidChangeActiveTextEditor` doesn't capture changes of untitled `document`s. This is because when the user
// change language id from `plaintext` to `typst` manually, the editor is not changed but only replacen with a new
// document with language id `typst`, in which case vscode doesn't trigger `onDidChangeActiveTextEditor`.
//
// FIXME1: we could do better by finding a way to handle the focus of `document` with only one handler.
// FIXME2: seems like we are also failing to capture changes of language id of titled `document`?
vscode.workspace.onDidOpenTextDocument((doc: vscode.TextDocument) => {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants