Feature/2122 editor focus #2955
Conversation
🦋 Changeset detectedLatest commit: c3e4794 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
dosubot
bot
added
size:L
|
thanks for that awesome PR. |
|
awesome! Does it work if you tear off a roo tab by clicking |
@KJ7LNW Screen.Recording.2025-04-26.at.11.58.41.1.mov |
felixAnhalt
force-pushed
the
feature/2122-editor-focus
branch
from
627e4af to
7a4e964
Compare
|
If you have multiple files in a single window/group, will edits by roo keep the tab position in the tabbar? |
I just set up multiple windows with multiple tabs and let it go for it. Screen.Recording.2025-04-26.at.22.11.43.mov |
I noticed that too. I am running your PR in my daily driver, and it really works great. Also noticed:
|
|
I've been editing in multiple Roo windows (same vscode instance/repo) and the editor window seems to pop up in unexpected locations. Could you see if An this case "Auto-approve Write" was disabled because I needed to approve any changes before letting it go, but I wanted it to open in the relevant task window. instead I think it opened in whatever window had focus. |
|
hey @KJ7LNW |
|
|
|
I tried to make tabs stay opened and make the diffview popup next to the original tab location. Works across groups, and for pinned tabs too (so that they dont get unpinned and stay in their original position), but that broke the "problems" detection for roo somehow. Maybe you can implement it? |
|
hey @seedlord I had a look at that. Therefore I assume we either need to find the correct |
Some of this will depend on what VS Code actually supports, but the following could be a useful consideration:
|
|
Hey // applyDiffTool.ts
const clineRef = cline.providerRef.deref()
const viewColumn = clineRef?.getViewColumn() ?? ViewColumn.Beside
// ClineProvider.ts
public getViewColumn(): vscode.ViewColumn {
const isViewPanel = this.view?.viewType === "roo-cline.TabPanelProvider"
if (!isViewPanel) {
return vscode.ViewColumn.Beside
}
// If the view is a WebviewPanel, return its viewColumn.
// This property is only set if the webview is in one of the editor view columns.
// Therefore, we can safely return it or default to beside.
return (this.view as vscode.WebviewPanel).viewColumn ?? vscode.ViewColumn.Beside
}Regarding the user disruption: I'm not aware of other options to use some kind of "open in new tab" behavior known from browsers for example. |
you might be hitting the limitations of the vs code API. you could try this:
I have done this many times successfully for things like shell integration. VScode is a big project and Roo has to read a bunch of files for a while to find out what you're looking for, but it does a pretty good job. |
|
That was a lot of fun.
Eventually I settled with temporarily saving a reference to Furthermore:
I tried to open it in the same tab group and that definitively steals focus. |
When I was testing (in a branch that I never created a pr for) I was able to open in the same tag group without stealing focus, and most of the time it even opened that tab in the window where Roo was, which seemed empirically to be caused by The entire diff is below, but I think this is the relevant part that got me close to that goal: + await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
+ preview: false,
+ viewColumn: vscode.ViewColumn.Active,
+ preserveFocus: true,
+ })
I agree with you: it should open in the same existing tab group without stealing focus, unless:
There are times when i really wish it would leave the split-diff window open ("Working Tree") after editing so I can review while it works on something else, but I know the number of tabs can also become too much clutter. I am sure there are cases when users want it one way or the other (indeed, I want it one way or the other depending on what type of work I am doing). So, this might be a useful configuration option: "close tabs after editing unless they were already open" critical: "unless they were already open" - sometimes windows closed and I was looking at something and I really wish it would not surprise me. Relatedly, there seems to be a behavior where existing diff windows are closed after editing, which I think you correctly pointed out as a problem with
To reiterate above: Intelligent use of auto-focus is a prime goal of this PR: it should open somewhere sane* without stealing focus, unless:
Where "sane" means that it does not steal my input, nor does it affect the view of the file I am currently looking at. Examples of "sane" in this case might mean:
Here is the entire diff that I worked on in case it is useful for reference: My old testing commitsFirst mostly working commitcommit b009fb71cd7da428780b186edd347ed7d0290b64
Author: Eric Wheeler
Date: Fri Apr 11 16:10:34 2025 -0700
feat: improve diff view focus handling
Add preserveFocus and viewColumn options to diff view operations
Switch to onDidChangeVisibleTextEditors for better editor tracking
diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts
index 0bf49485..412d275a 100644
--- a/src/integrations/editor/DiffViewProvider.ts
+++ b/src/integrations/editor/DiffViewProvider.ts
@@ -156,9 +156,20 @@ export class DiffViewProvider {
await updatedDocument.save()
}
- await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false })
- await this.closeAllDiffViews()
+ const currentEditor = vscode.window.activeTextEditor
+ await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
+ preview: false,
+ viewColumn: vscode.ViewColumn.Active,
+ preserveFocus: true,
+ })
+ if (currentEditor) {
+ await vscode.window.showTextDocument(currentEditor.document, {
+ viewColumn: currentEditor.viewColumn,
+ selection: currentEditor.selection,
+ })
+ }
+ await this.closeAllDiffViews()
/*
Getting diagnostics before and after the file edit is a better approach than
automatically tracking problems in real-time. This method ensures we only
@@ -280,15 +291,26 @@ export class DiffViewProvider {
arePathsEqual(tab.input.modified.fsPath, uri.fsPath),
)
if (diffTab && diffTab.input instanceof vscode.TabInputTextDiff) {
- const editor = await vscode.window.showTextDocument(diffTab.input.modified)
+ const currentEditor = vscode.window.activeTextEditor
+ const editor = await vscode.window.showTextDocument(diffTab.input.modified, {
+ viewColumn: vscode.ViewColumn.Active,
+ preserveFocus: true,
+ })
+ if (currentEditor) {
+ await vscode.window.showTextDocument(currentEditor.document, {
+ viewColumn: currentEditor.viewColumn,
+ selection: currentEditor.selection,
+ })
+ }
return editor
}
// Open new diff editor
return new Promise<vscode.TextEditor>((resolve, reject) => {
const fileName = path.basename(uri.fsPath)
const fileExists = this.editType === "modify"
- const disposable = vscode.window.onDidChangeActiveTextEditor((editor) => {
- if (editor && arePathsEqual(editor.document.uri.fsPath, uri.fsPath)) {
+ const disposable = vscode.window.onDidChangeVisibleTextEditors((editors) => {
+ const editor = editors.find((e) => arePathsEqual(e.document.uri.fsPath, uri.fsPath))
+ if (editor) {
disposable.dispose()
resolve(editor)
}
@@ -300,6 +322,7 @@ export class DiffViewProvider {
}),
uri,
`${fileName}: ${fileExists ? "Original ↔ Roo's Changes" : "New File"} (Editable)`,
+ { preserveFocus: true, viewColumn: vscode.ViewColumn.Active },
)
// This may happen on very slow machines ie project idx
setTimeout(() => {Second WIP commitcommit 7700b2fe44e64e8e8f0b79a18f3f0136ade99c53
Author: Eric Wheeler
Date: Fri Apr 25 20:02:02 2025 -0700
not working the way I want yet, needs further testing and troubleshooting
diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts
index 412d275a..958ccb5b 100644
--- a/src/integrations/editor/DiffViewProvider.ts
+++ b/src/integrations/editor/DiffViewProvider.ts
@@ -8,6 +8,7 @@ import { DecorationController } from "./DecorationController"
import * as diff from "diff"
import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
import stripBom from "strip-bom"
+import { ClineProvider } from "../../core/webview/ClineProvider"
export const DIFF_VIEW_URI_SCHEME = "cline-diff"
@@ -20,6 +21,7 @@ export class DiffViewProvider {
private relPath?: string
private newContent?: string
private activeDiffEditor?: vscode.TextEditor
+ private savedEditor?: { editor: vscode.TextEditor; selection: vscode.Selection }
private fadedOverlayController?: DecorationController
private activeLineController?: DecorationController
private streamedLines: string[] = []
@@ -156,17 +158,18 @@ export class DiffViewProvider {
await updatedDocument.save()
}
- const currentEditor = vscode.window.activeTextEditor
await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), {
preview: false,
viewColumn: vscode.ViewColumn.Active,
preserveFocus: true,
})
- if (currentEditor) {
- await vscode.window.showTextDocument(currentEditor.document, {
- viewColumn: currentEditor.viewColumn,
- selection: currentEditor.selection,
+ // Restore the editor that was active when diff was opened
+ if (this.savedEditor) {
+ await vscode.window.showTextDocument(this.savedEditor.editor.document, {
+ selection: this.savedEditor.selection,
})
+ // Clear saved editor after restoring
+ this.savedEditor = undefined
}
await this.closeAllDiffViews()
@@ -280,6 +283,15 @@ export class DiffViewProvider {
if (!this.relPath) {
throw new Error("No file path set")
}
+ const provider = ClineProvider.getVisibleInstance()
+ const preserveFocus = provider?.getValue("autoApprovalEnabled") && provider?.getValue("alwaysAllowWrite")
+ // Store current editor before opening diff
+ if (vscode.window.activeTextEditor) {
+ this.savedEditor = {
+ editor: vscode.window.activeTextEditor,
+ selection: vscode.window.activeTextEditor.selection,
+ }
+ }
const uri = vscode.Uri.file(path.resolve(this.cwd, this.relPath))
// If this diff editor is already open (ie if a previous write file was interrupted) then we should activate that instead of opening a new diff
const diffTab = vscode.window.tabGroups.all
@@ -294,11 +306,10 @@ export class DiffViewProvider {
const currentEditor = vscode.window.activeTextEditor
const editor = await vscode.window.showTextDocument(diffTab.input.modified, {
viewColumn: vscode.ViewColumn.Active,
- preserveFocus: true,
+ preserveFocus,
})
if (currentEditor) {
await vscode.window.showTextDocument(currentEditor.document, {
- viewColumn: currentEditor.viewColumn,
selection: currentEditor.selection,
})
} |
dosubot
bot
added
size:XL
|
I added the functionality to
I also added the checks for Auto-approve here. Anyways, regarding the focus preservation: I did not get it working that the tab is opened in the background inside the active tab group.
but at an earlier point of the procedure. |
daniel-lxs
moved this from PR [Draft / In Progress]
to PR [Needs Prelim Review]
in Roo Code Roadmap
hannesrudolph
added
PR - Needs Preliminary Review
and removed
PR - Draft / In Progress
labels
|
@felixAnhalt - You have 98 commits and +/- 4000 lines (!) which is going to be very difficult to review, especially because what commits look like at the beginning is not the same as what it looks like at the end. Please use Please watch this: If during the process you discover that this really should be provided as multiple PRs with different features that build on each other, then watch this one, too: Remember to create a backup branch before moving hunks around! I really like this feature and I want to see it merged, so it will help to make review easy for reviewers. For example:
|
|
I expected something like that response. For future reference, the backup branch is here. |
I just started using it and was pretty proficient within a few hours. Mostly it is a matter of figuring out what the hot keys are: since I had trouble finding it here is a hint: Also in case it is useful, this is my configuration: git:
autoStageResolvedConflicts: false
gui:
skipRewordInEditorWarning: true
customCommands:
- key: 'E'
description: 'Add empty commit'
context: 'commits'
command: 'git commit --allow-empty -m "empty commit"'
loadingText: 'Committing empty commit...' |
|
Hi @felixAnhalt, thanks for this significant contribution and for tackling the editor focus issue. This is a substantial PR with a lot of great work. After reviewing the changes, I have some concerns about the scope. This PR currently combines several major changes:
While the new architecture is a definite improvement, bundling this many changes into a single PR makes it difficult to review thoroughly and increases the risk of introducing unintended side effects. For example, during testing, it appears that when We've also noted a few smaller things, which are easy fixes but highlight the challenge of catching everything in a PR of this size. Would you be open to splitting this PR into smaller, more focused ones? For example:
This would make the review process much smoother and help get these valuable changes merged more quickly. Thanks again for your work on this! |
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Changes Requested]
in Roo Code Roadmap
hannesrudolph
added
PR - Changes Requested
and removed
PR - Needs Preliminary Review
labels
|
Hey @daniel-lxs My process will be: Create/Update relevant issues for these scopes and reference this PR. Thank you for your patience :) |
One PR may end up depending on a previous, so if you decide to stack branches (with Lazygit), please put a empty commit message at the beginning of each branch so that it is easier for reviewers to tell where one branch finishes and the other picked up. For example: 
...
|
|
@felixAnhalt let us know when you split the pr's/issues (at least logically) - would be happy to work on this as well |
|
hey @ccrvlh for the initial autofocus issue #2122 there is already a new PR here. for the two options related to the closing of tabs ( I wanted to touch the Then there are the options to granurarily decide on editing behavior still open:
You could look at this branch as a reference implementation on how I extracted relevant code snippets from this current branch into their own. I also moved Another side info: the whole part with the If you wanna coordinate efforts, please add me on discord. |
|
here you can find the implementation for I will create an issue and PR for that tomorrow. |
|
I think I have extracted all the core ideas of this PR into their respective issues. To reiterate:
My proposed order of implementation:
#6003 is entirely separate. If everyone is fine with it, I would close this one here. |
|
hey y'all. you guys are clearly on top of it, so i'll just jump in to share my perspective as user. going through the pr, my general comments:
you guys have been working on it for a while now, and there's a lot involved in the changes - so lmk if there's a specific topic that hasn't been explored yet about those changes, or maybe a specific logic for the organization and i'll be happy to help. |
|
regarding
you could have a look at this PR :)
Good catch! Correct in this case means "the tab group the roo instance spawning the task was in".
I would argue it is tangentially related because we actually have to take focus to the document at the end of the tab list for a couple of ms (which steals focus). More about that here.
Nice catch again. autoCloseRooTabs == close tabs opened by roo; autoCloseAllRooTabs == close all tabs touched by roo. More about that here.. |
github-project-automation
bot
moved this from PR [Changes Requested]
to Done
in Roo Code Roadmap
github-project-automation
bot
moved this from PR [Greenlit]
to Done
in Roo Code Roadmap
8 participants
Context
I added an editable setting to allow users to preserve focus on the tab they are in while roo code is opening new tabs to do what it has to do.
Fixes #2122.
Implementation
Added
roo-cline.diffViewAutoFocusin the VSCodesettings.jsonas well as all the attributes and types related to that.Added message stuff for frontend and backend communication. Adjuste behavior of
DiffCheckViewerto preserve editor focus or don't, depending on the setting set. Left this option with defaulttruefor an opt-out experience, so that the previous behavior is suststained if one doesn't excplicitely wants to change it.Added lots of settings to control editing behavior.
New Settings Page:
If diff editing is enabled:
Option to close newly created and previously not opened roo tabs
Option to close all tabs roo touched
Option to focus the diff view
Option to open the diff view in the correct window
Option to open the tabs at the end of the tab list
Screenshots
New Settings Page
Auto Focus Enabled
auto_focus_enabled.mov
Auto Focus Disabled
auto_focus_disabled.mov
Auto Closing of Tabs
auto_close.1.mov
Standard behavior: 00:00 - 00:18
Close All Opened Tabs with example tab already open (shouldn't be closed): 00:29 - 00:45
Close All Opened Tabs w/o example tab opened (both should be closed): 05:02 - 05:07 (quite fast!)
How to Test
Follow the steps from the video.
En- disable the setting once.
Execute a with- and without it enabled to verify that it steals focus when autoFocus is enabed and doesn't if not.
Get in Touch
Discord handle: icy_ice_pls
Anyways, here's GitHubs automatic summary:
This pull request introduces several changes to enhance the functionality of the Roo Code editor, including the addition of new settings, the replacement of the
DiffViewProviderwith a more versatileEditingProvider, and the removal of legacy migration logic for diff settings. The updates aim to improve user experience by providing more customization options and streamlining the codebase.New Features and Settings:
.changeset/large-snakes-suffer.md).diffViewAutoFocus,autoCloseRooTabs, andopenTabsAtEndOfListinglobalSettingsSchema(packages/types/src/global-settings.ts).Migration and Cleanup:
diffSettingsMigratedlogic and themigrateDiffSettingsmethod fromProviderSettingsManager(src/core/config/ProviderSettingsManager.ts). [1] [2]diffEnabledandfuzzyMatchThresholdfrombaseProviderSettingsSchemaas they are no longer required (packages/types/src/provider-settings.ts).Refactoring for Editing Provider:
DiffViewProviderwithIEditingProviderthroughout theTaskclass, enabling more flexible editing operations (src/core/task/Task.ts). [1] [2]Taskto useEditingProviderFactoryfor creating and resetting editing providers (src/core/task/Task.ts). [1] [2]Test Updates:
diffViewProviderwitheditingProviderin mock objects and assertions (src/core/tools/__tests__/writeToFileTool.spec.ts). [1] [2]These changes collectively improve the extensibility and maintainability of the Roo Code editor while enhancing the user experience.
Important
This PR adds new settings to manage editor focus and tab behavior in Roo Code, refactors editing providers, and updates the UI and tests accordingly.
roo-cline.diffViewAutoFocussetting to control editor focus behavior when opening new tabs.settings.jsonfordiffViewAutoFocus,autoCloseRooTabs,openTabsAtEndOfList, and more.globalSettingsSchemainglobal-settings.tsto include new settings.DiffViewProviderwithIEditingProviderinTask.tsfor more flexible editing operations.EditingProviderFactoryto create appropriate editing providers based on settings.diffSettingsMigratedlogic fromProviderSettingsManager.ts.FileEditingOptionscomponent for new settings in the UI.SettingsView.tsxto include new file editing options.EditingProviderFactory,FileWriter, andUserInteractionProvider.This description was created by
for b4f213e. You can customize this summary. It will automatically update as commits are pushed.