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

Adding overtype mode #233188

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

Adding overtype mode #233188

wants to merge 40 commits into from

Conversation

aiday-mar
Copy link
Contributor

@aiday-mar aiday-mar commented Nov 6, 2024

fixes #1012

@aiday-mar aiday-mar self-assigned this Nov 6, 2024
@aiday-mar aiday-mar changed the title Adding setting to enable overtype mode [DRAFT]: Adding setting to enable overtype mode Nov 6, 2024
@aiday-mar aiday-mar changed the title [DRAFT]: Adding setting to enable overtype mode Adding overtype mode Nov 13, 2024
@aiday-mar aiday-mar marked this pull request as ready for review November 13, 2024 16:16
@aiday-mar aiday-mar marked this pull request as draft November 13, 2024 16:16
@aiday-mar aiday-mar marked this pull request as ready for review November 13, 2024 16:43
@aiday-mar aiday-mar enabled auto-merge (squash) November 13, 2024 16:43
@vs-code-engineering vs-code-engineering bot added this to the November 2024 milestone Nov 13, 2024
@aiday-mar
Copy link
Contributor Author

Just discovered there are unit test failures, will look into this

@aiday-mar
Copy link
Contributor Author

Test failure was happening because I forgot to reset the input mode to 'insert' from 'overtype' on suite teardown.

@GitMensch
Copy link
Contributor

Without inspecting the code changes it looks to me like this PR could use some squashing of commits. One big is possibly too big, but 26 commits seems too much.

Apart from this, will documentation added separately or should this PR also include some?

@aiday-mar
Copy link
Contributor Author

Hi this PR is set to squash merge automatically when approved. As for the documentation I am waiting on my colleague's review.

Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

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

Nice! I left some comments.

src/vs/base/common/inputMode.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorStatus.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorStatus.ts Outdated Show resolved Hide resolved
src/vs/workbench/browser/parts/editor/editorActions.ts Outdated Show resolved Hide resolved
@@ -5951,6 +5988,9 @@ export const EditorOptions = {
tags: ['preview']
}
)),
overtypeOnPaste: register(new EditorBooleanOption(
EditorOption.overtypeOnPaste, 'overtypeOnPaste', true
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this could be implemented differently, maybe via the paste follow-up action that is rendered? That widget allows picking between different paste outcomes when pasting for example code blocks in TypeScript or an image link in markdown.

Copy link
Contributor Author

@aiday-mar aiday-mar Nov 20, 2024

Choose a reason for hiding this comment

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

Following our discussion, I think we can implement this as a follow-up action if there is demand for it. Out of the three editors that I have analyzed that allow overtyping on paste, only one seems to have a setting to toggle between the different paste modes and it is the extension Overtype for VS Code. I wonder if we should keep our paste on overtype setting just to mirror this extension which seems to be the go-to extension for our overtype users.

In addition to this, half of the editors I analyzed do not offer overtype on paste mode and pasting just inserts, so I am thinking of keeping the setting but setting it to false.

src/vs/editor/common/cursor/cursorTypeEditOperations.ts Outdated Show resolved Hide resolved
src/vs/editor/browser/viewParts/viewCursors/viewCursor.ts Outdated Show resolved Hide resolved
src/vs/editor/common/config/editorOptions.ts Show resolved Hide resolved
src/vs/editor/common/commands/replaceCommand.ts Outdated Show resolved Hide resolved
src/vs/editor/common/editorCommon.ts Outdated Show resolved Hide resolved
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.

Insert key doesn't switch to overtype/overwrite mode
3 participants