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

Add smart paste terminal feature #231178

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5a376d4
229280: Added a terminal feature setting to allow smart paste
Parasaran-Python Oct 9, 2024
8011c39
229280: Smart paste utils added to process paths based on shell type
Parasaran-Python Oct 10, 2024
b77da91
229280: Processing text before pasting if its a path
Parasaran-Python Oct 11, 2024
ab99247
229280: Fixed failing unit test cases
Parasaran-Python Oct 12, 2024
75be37f
229280: Corrected the shell type for pwsh
Parasaran-Python Oct 12, 2024
83cae85
229280: Added test cases for smart paste
Parasaran-Python Oct 12, 2024
b0f603f
229280: Corrected regex for windows paths
Parasaran-Python Oct 12, 2024
fe056ef
229280: AllowSmartPaste -> EnableSmartPaste
Parasaran-Python Oct 14, 2024
adef711
229280: Moved enableSmartPaste setting to terminalClipboardConfig
Parasaran-Python Oct 14, 2024
ada8363
229280: Moved smartPasteUtils to a separate file
Parasaran-Python Oct 14, 2024
dd59152
229280: Improved doc comments and func name
Parasaran-Python Oct 14, 2024
3f64e3b
229280: Improved var naming convention
Parasaran-Python Oct 14, 2024
56042b8
229280: Added non capturing groups
Parasaran-Python Oct 14, 2024
c8a1efb
229280: Wrap and escape paths
Parasaran-Python Oct 14, 2024
066ef31
229280: Importing SmartPasteUtils
Parasaran-Python Oct 14, 2024
3523384
229280: Added doc comment for smartpasteutils class
Parasaran-Python Oct 14, 2024
5fc1908
229280_changes: Adding shellType param to IBaseTerminalInstance inter…
Parasaran-Python Oct 14, 2024
8d798bb
229280: Wrapped func params in an object and defaulted param to false
Parasaran-Python Oct 14, 2024
2a5a2fb
229280_changes: Added more shell types
Parasaran-Python Oct 15, 2024
ba11c25
229280: Handling relative paths in terminal smartpaste
Parasaran-Python Oct 17, 2024
a060f43
229280: Added tests for relative paths
Parasaran-Python Oct 17, 2024
7f14757
229280: Handling ctrl+v of pwsh in the main handler of terminal clipb…
Parasaran-Python Oct 19, 2024
8142c4b
229280: Preventing ctrl+v on single line paste
Parasaran-Python Oct 19, 2024
9e6d2a7
229280: Escaping with a character based on the shell type
Parasaran-Python Oct 19, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { MicrotaskDelay } from '../../../../base/common/symbols.js';
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
import { TerminalCapabilityStore } from '../../../../platform/terminal/common/capabilities/terminalCapabilityStore.js';
import { IMergedEnvironmentVariableCollection } from '../../../../platform/terminal/common/environmentVariable.js';
import { ITerminalBackend } from '../../../../platform/terminal/common/terminal.js';
Parasaran-Python marked this conversation as resolved.
Show resolved Hide resolved
import { ITerminalBackend, TerminalShellType } from '../../../../platform/terminal/common/terminal.js';
Copy link

Choose a reason for hiding this comment

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

Suggested change
import { ITerminalBackend, TerminalShellType } from '../../../../platform/terminal/common/terminal.js';
import { ITerminalBackend, TerminalShellType } จาก ITerminalBackend,'../../../../platform/terminal/common/terminal.js';

import { IDetachedTerminalInstance, IDetachedXTermOptions, IDetachedXtermTerminal, ITerminalContribution, IXtermAttachToElementOptions } from './terminal.js';
import { TerminalExtensionsRegistry } from './terminalExtensions.js';
import { TerminalWidgetManager } from './widgets/widgetManager.js';
Expand Down Expand Up @@ -72,6 +72,7 @@ export class DetachedTerminal extends Disposable implements IDetachedTerminalIns
}
});
}
Copy link

Choose a reason for hiding this comment

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

Duplicate of #

shellType: TerminalShellType | undefined;
Copy link

Choose a reason for hiding this comment

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

Suggested change
shellType: TerminalShellType | undefined;
shellType: TerminalShellType | undefined;


get selection(): string | undefined {
return this._xterm && this.hasSelection() ? this._xterm.raw.getSelection() : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,22 +133,11 @@ Registry.as<IViewsRegistry>(ViewContainerExtensions.ViewsRegistry).registerViews
// Register actions
registerTerminalActions();

const enum Constants {
Copy link

Choose a reason for hiding this comment

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

ส่งออก const enum ค่าคงที่ {

export const enum Constants {
Copy link

Choose a reason for hiding this comment

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

shellType: TerminalShellType | undefined;

Suggested change
export const enum Constants {
export const enum Constants {

/** The text representation of `^<letter>` is `'A'.charCodeAt(0) + 1`. */
CtrlLetterOffset = 64
}

// An extra Windows-only ctrl+v keybinding is used for pwsh that sends ctrl+v directly to the
Copy link

Choose a reason for hiding this comment

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

// แมปคีย์ไบนดิ้งบางส่วนใน pwsh ไปยังคีย์ที่ไม่ได้ใช้ซึ่งจะถูกจัดการโดยตัวจัดการ PSReadLine ใน

// shell, this gets handled by PSReadLine which properly handles multi-line pastes. This is
Copy link

Choose a reason for hiding this comment

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

Suggested change
// shell, this gets handled by PSReadLine which properly handles multi-line pastes. This is
// shell integration script. This allows keystrokes that cannot be sent via VT sequences to work.

// disabled in accessibility mode as PowerShell does not run PSReadLine when it detects a screen
Copy link

Choose a reason for hiding this comment

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

// reader. This works even when clipboard.readText is not supported.
if (isWindows) {
registerSendSequenceKeybinding(String.fromCharCode('V'.charCodeAt(0) - Constants.CtrlLetterOffset), { // ctrl+v
when: ContextKeyExpr.and(TerminalContextKeys.focus, ContextKeyExpr.equals(TerminalContextKeyStrings.ShellType, GeneralShellType.PowerShell), CONTEXT_ACCESSIBILITY_MODE_ENABLED.negate()),
Copy link
Contributor Author

@Parasaran-Python Parasaran-Python Oct 19, 2024

Choose a reason for hiding this comment

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

@Tyriar I have removed this and handled the ctrl+v action in "src\vs\workbench\contrib\terminalContrib\clipboard\browser\terminalClipboard.ts" itself,

this used to handle only ctrl+v action on pwsh, if the user tries to paste using right mouse click the multi line paste didn't used to work properly (Not sure if this is a bug or an expected behavior),

Now with the changes I've made the right mouse click also works as expected

It's just that we get the multi line paste alert on ctrl+v as well, which in my opinion shouldn't be an issue as this brings in consistency in the behavior.

image

primary: KeyMod.CtrlCmd | KeyCode.KeyV
});
}

// Map certain keybindings in pwsh to unused keys which get handled by PSReadLine handlers in the
// shell integration script. This allows keystrokes that cannot be sent via VT sequences to work.
// See https://github.com/microsoft/terminal/issues/879#issuecomment-497775007
Expand Down
5 changes: 5 additions & 0 deletions src/vs/workbench/contrib/terminal/browser/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ export interface IBaseTerminalInstance {
*/
readonly selection: string | undefined;

/**
* The shell type of the terminal.
Copy link

Choose a reason for hiding this comment

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

Suggested change
* The shell type of the terminal.
* The shell type of the terminal.

*/
Copy link

Choose a reason for hiding this comment

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

Suggested change
*/
*/

readonly shellType: TerminalShellType | undefined;
Copy link

Choose a reason for hiding this comment

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

Suggested change
readonly shellType: TerminalShellType | undefined;
readonly shellType: TerminalShellType | undefined;


Copy link

Choose a reason for hiding this comment

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

Suggested change

/**
* Check if anything is selected in terminal.
*/
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/contrib/terminal/terminalContribExports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { TerminalAccessibilityCommandId, defaultTerminalAccessibilityCommandsToS
import { terminalAccessibilityConfiguration } from '../terminalContrib/accessibility/common/terminalAccessibilityConfiguration.js';
import { terminalAutoRepliesConfiguration } from '../terminalContrib/autoReplies/common/terminalAutoRepliesConfiguration.js';
import { terminalInitialHintConfiguration } from '../terminalContrib/chat/common/terminalInitialHintConfiguration.js';
import { terminalClipboardConfiguration } from '../terminalContrib/clipboard/common/terminalClipboardConfiguration.js';
Copy link

Choose a reason for hiding this comment

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

Suggested change
import { terminalClipboardConfiguration } from '../terminalContrib/clipboard/common/terminalClipboardConfiguration.js';
import { terminalClipboardConfiguration } from '../terminalContrib/clipboard/common/terminalClipboardConfiguration.js';

import { terminalCommandGuideConfiguration } from '../terminalContrib/commandGuide/common/terminalCommandGuideConfiguration.js';
import { TerminalDeveloperCommandId } from '../terminalContrib/developer/common/terminal.developer.js';
import { defaultTerminalFindCommandToSkipShell } from '../terminalContrib/find/common/terminal.find.js';
Expand Down Expand Up @@ -46,6 +47,7 @@ export const terminalContribConfiguration: IConfigurationNode['properties'] = {
...terminalSuggestConfiguration,
...terminalTypeAheadConfiguration,
...terminalZoomConfiguration,
...terminalClipboardConfiguration
};

Copy link

Choose a reason for hiding this comment

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

... การกำหนดค่าเทอร์มินัลคลิปบอร์ด

// Export commands to skip shell from terminalContrib - this is an exception to the eslint rule
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { OS } from '../../../../../base/common/platform.js';
import { detectLinks } from '../../links/browser/terminalLinkParsing.js';
import { fallbackMatchers } from '../../links/browser/terminalLocalLinkDetector.js';


/**
* Manages all the terminal smart paste related operations
*/
export class SmartPasteUtils {

private static _detectLink(text: string): boolean {
const links = detectLinks(text, OS);

/* Check the first link only */
if (links.length > 0 && links[0]?.path?.text === text) {
return true;
}

return false;
}

private static _matchFallbackMatchers(text: string) {
let hasMatchedFallbackMatchers = false;

fallbackMatchers.forEach((regexPattern) => {
if (regexPattern.test(text)) {
hasMatchedFallbackMatchers = true;
return;
}
});
return hasMatchedFallbackMatchers;
}

/**
* Check if the input string looks like a path
* @param text input string, returns true if it looks like a path
*/
static isPathLike(text: string): boolean {
// Regex to detect common path formats

const windowsPathPattern = /^[a-zA-Z]:(?:\\|\/)/; // Windows absolute path
const windowsUNCPathPattern = /^\\\\/; // Windows UNC path
const unixPathPattern = /^\/|(?:\w+\/)/; // Unix/Linux/macOS paths

return windowsPathPattern.test(text) ||
windowsUNCPathPattern.test(text) ||
unixPathPattern.test(text) ||
/* Checks below are to validate if the path is relative */
this._detectLink(text) ||
this._matchFallbackMatchers(text);
}

/**
* Wraps the input path in " and escapes the " in the path
* @param path input path which needs to be wrapped in "
*/
static wrapAndEscapePath(path: string, escapeChar: string): string {
// Escape double quotes in the path
const escapedPath = path.replace(/"/g, `${escapeChar}"`);
// Wrap the escaped path in double quotes
return `"${escapedPath}"`;
}

/**
* Handles smartPaste for paths depending on the type of the terminal
* @param text the string that's going to be pasted on the terminal
* @param shellType the type of terminal on which the paste operation was done
*/
static handleSmartPaste(text: string, shellType: string): string {
if (!this.isPathLike(text)) {
return text; // Return the string as is if it's not detected as a path
}

switch (shellType) {
case 'gitbash':
case 'zsh':
case 'tmux':
case 'fish':
case 'bash':
{
// Escape backslashes and wrap in double quotes if necessary
const escapedPath = text.replace(/\\/g, '\\\\');
if (text.includes(' ')) {
return this.wrapAndEscapePath(escapedPath, '\\');
}
return escapedPath;
}

case 'pwsh':
if (text.includes(' ')) {
return this.wrapAndEscapePath(text, '`');
}
return text;
case 'cmd':
// Simply wrap in quotes if spaces are present
if (text.includes(' ')) {
return this.wrapAndEscapePath(text, '^');
}
return text;

default:
return text; // If shell type is unknown, return text as is
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,17 @@ export class TerminalClipboardContribution extends Disposable implements ITermin
return;
}

let shellType = '';

Copy link

Choose a reason for hiding this comment

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

Suggested change

shellType = this._ctx.instance.shellType ?? '';

let currentText = value;
const shouldPasteText = await this._instantiationService.invokeFunction(shouldPasteTerminalText, currentText, this._xterm?.raw.modes.bracketedPasteMode);
const shouldPasteText = await this._instantiationService.invokeFunction(
shouldPasteTerminalText,
currentText,
this._xterm?.raw.modes.bracketedPasteMode,
shellType
);
if (!shouldPasteText) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,28 @@ import { IConfigurationService } from '../../../../../platform/configuration/com
import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js';
import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js';
import { TerminalSettingId } from '../../../../../platform/terminal/common/terminal.js';
import { Constants } from '../../../terminal/browser/terminal.contribution.js';
import { TerminalClipboardSettingId } from '../common/terminalClipboardConfiguration.js';
Copy link

Choose a reason for hiding this comment

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

Suggested change
import { TerminalClipboardSettingId } from '../common/terminalClipboardConfiguration.js';
import { TerminalClipboardSettingId } from '../common/terminalClipboardConfiguration.js';

import { SmartPasteUtils } from './smartPasteUtils.js';

export async function shouldPasteTerminalText(accessor: ServicesAccessor, text: string, bracketedPasteMode: boolean | undefined): Promise<boolean | { modifiedText: string }> {
export async function shouldPasteTerminalText(accessor: ServicesAccessor, text: string, bracketedPasteMode: boolean | undefined, shellType: string): Promise<boolean | { modifiedText: string }> {
const configurationService = accessor.get(IConfigurationService);
const dialogService = accessor.get(IDialogService);

// If the clipboard has only one line, a warning should never show
Copy link

Choose a reason for hiding this comment

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

Suggested change
// If the clipboard has only one line, a warning should never show
// If the clipboard has only one line, a warning should never show

const textForLines = text.split(/\r?\n/);

const isSmartPasteAllowed = configurationService.getValue(TerminalClipboardSettingId.EnableSmartPaste);
Copy link

Choose a reason for hiding this comment

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

Suggested change
const isSmartPasteAllowed = configurationService.getValue(TerminalClipboardSettingId.EnableSmartPaste);
const isSmartPasteAllowed = configurationService.getValue(TerminalClipboardSettingId.EnableSmartPaste);

// If the string is a path process it depending on the shell type
// multi line strings aren't handled
let modifiedText = text;

if (isSmartPasteAllowed) {
modifiedText = SmartPasteUtils.handleSmartPaste(text, shellType);
}

if (textForLines.length === 1) {
return true;
return text === modifiedText ? true : { modifiedText: modifiedText };
Copy link

Choose a reason for hiding this comment

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

Suggested change
return text === modifiedText ? true : { modifiedText: modifiedText };
return text === modifiedText ? true : { modifiedText: modifiedText };

}

// Get config value
Expand Down Expand Up @@ -94,8 +107,14 @@ export async function shouldPasteTerminalText(accessor: ServicesAccessor, text:
return false;
}

if (result.confirmed && checkboxChecked) {
await configurationService.updateValue(TerminalSettingId.EnableMultiLinePasteWarning, 'never');
if (result.confirmed) {
/* Send ctrl+v to PSReadline if its a pwsh instance */
if (shellType === 'pwsh' && !result.singleLine) {
return { modifiedText: String.fromCharCode('V'.charCodeAt(0) - Constants.CtrlLetterOffset) };
}
if (checkboxChecked) {
await configurationService.updateValue(TerminalSettingId.EnableMultiLinePasteWarning, 'never');
}
}

if (result.singleLine) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import type { IStringDictionary } from '../../../../../base/common/collections.js';
import { localize } from '../../../../../nls.js';
import type { IConfigurationPropertySchema } from '../../../../../platform/configuration/common/configurationRegistry.js';

export const enum TerminalClipboardSettingId {
EnableSmartPaste = 'terminal.integrated.EnableSmartPaste',
}

export interface ITerminalClipboardConfiguration {
enableSmartPaste: boolean;
}

export const terminalClipboardConfiguration: IStringDictionary<IConfigurationPropertySchema> = {
[TerminalClipboardSettingId.EnableSmartPaste]: {
markdownDescription: localize('terminal.integrated.enableSmartPaste', "Whether or not to allow smart paste to automatically wrap file path with double quotes"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We dont just wrap the path with ", we just escape the \ if there are no spaces, need a text that's more clear

type: 'boolean',
default: false
},
};
Loading