Skip to content

Commit 1f7b7ef

Browse files
Copilotanthonykim1Tyriar
authored
Add leading space to shell activation commands to prevent history bloating (#1143)
Activation commands (`source /path/to/activate`) were being saved to bash history on each terminal open, bloating history and making reverse-i-search unusable. ### Changes - **Added `shellsWithLeadingSpaceHistorySupport`** - Shells supporting `HISTCONTROL=ignorespace` (bash) or `setopt HIST_IGNORE_SPACE` (zsh): `bash`, `zsh`, `gitbash` - **Modified `getShellCommandAsString`** - Prepends space for supported shells ```typescript // Before: "source /path/to/venv/bin/activate" // After: " source /path/to/venv/bin/activate" ``` ### Shell investigation | Shell | Leading space support | |-------|----------------------| | bash, zsh, gitbash | ✅ Implemented | | pwsh | Requires custom `AddToHistoryHandler` - not default | | fish, sh, ksh, csh, nu | Not supported | <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Bash activation command without proceeding space bloating bash-history</issue_title> > <issue_description>The activation of the venv is performed by command `source $venv_path` without space at the start. This result in this command stored multiple time in bash-history, bloating and overwriting it. This quickly makes reverse-i-search unusable. > > Please use ` source $venv_path` instead.</issue_description> > > <agent_instructions>Investigate if we can do the same for other shells as well. Shell specific </agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #604 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: anthonykim1 <62267334+anthonykim1@users.noreply.github.com> Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
1 parent f165623 commit 1f7b7ef

File tree

2 files changed

+139
-24
lines changed

2 files changed

+139
-24
lines changed

src/features/terminal/shells/common/shellUtils.ts

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,45 @@ import { getConfiguration } from '../../../../common/workspace.apis';
44
import { ShellConstants } from '../../../common/shellConstants';
55
import { quoteArgs } from '../../../execution/execUtils';
66

7-
function getCommandAsString(command: PythonCommandRunConfiguration[], shell: string, delimiter: string): string {
7+
/**
8+
* Shells that support a leading space to prevent command from being saved in history.
9+
* - Bash: When HISTCONTROL contains 'ignorespace' or 'ignoreboth'
10+
* - Zsh: When setopt HIST_IGNORE_SPACE is enabled
11+
* - Git Bash: Uses bash under the hood, same behavior as Bash
12+
*/
13+
export const shellsWithLeadingSpaceHistorySupport = new Set([
14+
ShellConstants.BASH,
15+
ShellConstants.ZSH,
16+
ShellConstants.GITBASH,
17+
]);
18+
19+
const defaultShellDelimiter = '&&';
20+
const shellDelimiterByShell = new Map<string, string>([
21+
[ShellConstants.PWSH, ';'],
22+
[ShellConstants.NU, ';'],
23+
[ShellConstants.FISH, '; and'],
24+
]);
25+
26+
export function getShellCommandAsString(shell: string, command: PythonCommandRunConfiguration[]): string {
27+
const delimiter = shellDelimiterByShell.get(shell) ?? defaultShellDelimiter;
828
const parts = [];
929
for (const cmd of command) {
1030
const args = cmd.args ?? [];
1131
parts.push(quoteArgs([normalizeShellPath(cmd.executable, shell), ...args]).join(' '));
1232
}
13-
if (shell === ShellConstants.PWSH) {
14-
if (parts.length === 1) {
15-
return parts[0];
16-
}
17-
return parts.map((p) => `(${p})`).join(` ${delimiter} `);
33+
34+
let commandStr = parts.join(` ${delimiter} `);
35+
if (shell === ShellConstants.PWSH && parts.length > 1) {
36+
commandStr = parts.map((p) => `(${p})`).join(` ${delimiter} `);
1837
}
19-
return parts.join(` ${delimiter} `);
20-
}
2138

22-
export function getShellCommandAsString(shell: string, command: PythonCommandRunConfiguration[]): string {
23-
switch (shell) {
24-
case ShellConstants.PWSH:
25-
return getCommandAsString(command, shell, ';');
26-
case ShellConstants.NU:
27-
return getCommandAsString(command, shell, ';');
28-
case ShellConstants.FISH:
29-
return getCommandAsString(command, shell, '; and');
30-
case ShellConstants.BASH:
31-
case ShellConstants.SH:
32-
case ShellConstants.ZSH:
33-
34-
case ShellConstants.CMD:
35-
case ShellConstants.GITBASH:
36-
default:
37-
return getCommandAsString(command, shell, '&&');
39+
// Add a leading space for shells that support history ignore with leading space.
40+
// This prevents the activation command from being saved in bash/zsh history
41+
// when HISTCONTROL=ignorespace (bash) or setopt HIST_IGNORE_SPACE (zsh) is set.
42+
if (shellsWithLeadingSpaceHistorySupport.has(shell)) {
43+
return ` ${commandStr}`;
3844
}
45+
return commandStr;
3946
}
4047

4148
export function normalizeShellPath(filePath: string, shellType?: string): string {

src/test/features/terminal/shells/common/shellUtils.unit.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import * as assert from 'assert';
2+
import { PythonCommandRunConfiguration } from '../../../../../api';
3+
import { ShellConstants } from '../../../../../features/common/shellConstants';
24
import {
35
extractProfilePath,
6+
getShellCommandAsString,
47
PROFILE_TAG_END,
58
PROFILE_TAG_START,
9+
shellsWithLeadingSpaceHistorySupport,
610
} from '../../../../../features/terminal/shells/common/shellUtils';
711

812
suite('Shell Utils', () => {
@@ -77,4 +81,108 @@ suite('Shell Utils', () => {
7781
assert.strictEqual(result, expectedPath);
7882
});
7983
});
84+
85+
suite('shellsWithLeadingSpaceHistorySupport', () => {
86+
test('should include bash, zsh, and gitbash', () => {
87+
assert.ok(shellsWithLeadingSpaceHistorySupport.has(ShellConstants.BASH));
88+
assert.ok(shellsWithLeadingSpaceHistorySupport.has(ShellConstants.ZSH));
89+
assert.ok(shellsWithLeadingSpaceHistorySupport.has(ShellConstants.GITBASH));
90+
});
91+
92+
test('should not include shells without leading space history support', () => {
93+
assert.ok(!shellsWithLeadingSpaceHistorySupport.has(ShellConstants.PWSH));
94+
assert.ok(!shellsWithLeadingSpaceHistorySupport.has(ShellConstants.CMD));
95+
assert.ok(!shellsWithLeadingSpaceHistorySupport.has(ShellConstants.FISH));
96+
assert.ok(!shellsWithLeadingSpaceHistorySupport.has(ShellConstants.SH));
97+
assert.ok(!shellsWithLeadingSpaceHistorySupport.has(ShellConstants.NU));
98+
});
99+
});
100+
101+
suite('getShellCommandAsString', () => {
102+
const sampleCommand: PythonCommandRunConfiguration[] = [
103+
{ executable: 'source', args: ['/path/to/activate'] },
104+
];
105+
106+
suite('leading space for history ignore', () => {
107+
test('should add leading space for bash commands', () => {
108+
const result = getShellCommandAsString(ShellConstants.BASH, sampleCommand);
109+
assert.ok(result.startsWith(' '), 'Bash command should start with a leading space');
110+
assert.ok(result.includes('source'), 'Command should contain source');
111+
});
112+
113+
test('should add leading space for zsh commands', () => {
114+
const result = getShellCommandAsString(ShellConstants.ZSH, sampleCommand);
115+
assert.ok(result.startsWith(' '), 'Zsh command should start with a leading space');
116+
assert.ok(result.includes('source'), 'Command should contain source');
117+
});
118+
119+
test('should add leading space for gitbash commands', () => {
120+
const result = getShellCommandAsString(ShellConstants.GITBASH, sampleCommand);
121+
assert.ok(result.startsWith(' '), 'Git Bash command should start with a leading space');
122+
assert.ok(result.includes('source'), 'Command should contain source');
123+
});
124+
125+
test('should not add leading space for pwsh commands', () => {
126+
const result = getShellCommandAsString(ShellConstants.PWSH, sampleCommand);
127+
assert.ok(!result.startsWith(' '), 'PowerShell command should not start with a leading space');
128+
});
129+
130+
test('should not add leading space for cmd commands', () => {
131+
const result = getShellCommandAsString(ShellConstants.CMD, sampleCommand);
132+
assert.ok(!result.startsWith(' '), 'CMD command should not start with a leading space');
133+
});
134+
135+
test('should not add leading space for fish commands', () => {
136+
const result = getShellCommandAsString(ShellConstants.FISH, sampleCommand);
137+
assert.ok(!result.startsWith(' '), 'Fish command should not start with a leading space');
138+
});
139+
140+
test('should not add leading space for sh commands', () => {
141+
const result = getShellCommandAsString(ShellConstants.SH, sampleCommand);
142+
assert.ok(!result.startsWith(' '), 'SH command should not start with a leading space');
143+
});
144+
145+
test('should not add leading space for nu commands', () => {
146+
const result = getShellCommandAsString(ShellConstants.NU, sampleCommand);
147+
assert.ok(!result.startsWith(' '), 'Nu command should not start with a leading space');
148+
});
149+
150+
test('should not add leading space for unknown shells', () => {
151+
const result = getShellCommandAsString('unknown', sampleCommand);
152+
assert.ok(!result.startsWith(' '), 'Unknown shell command should not start with a leading space');
153+
});
154+
});
155+
156+
suite('command formatting', () => {
157+
test('should format multiple commands with && for bash', () => {
158+
const multiCommand: PythonCommandRunConfiguration[] = [
159+
{ executable: 'source', args: ['/path/to/init'] },
160+
{ executable: 'conda', args: ['activate', 'myenv'] },
161+
];
162+
const result = getShellCommandAsString(ShellConstants.BASH, multiCommand);
163+
assert.ok(result.includes('&&'), 'Bash should use && to join commands');
164+
assert.ok(result.startsWith(' '), 'Bash command should start with a leading space');
165+
});
166+
167+
test('should format multiple commands with ; for pwsh', () => {
168+
const multiCommand: PythonCommandRunConfiguration[] = [
169+
{ executable: 'source', args: ['/path/to/init'] },
170+
{ executable: 'conda', args: ['activate', 'myenv'] },
171+
];
172+
const result = getShellCommandAsString(ShellConstants.PWSH, multiCommand);
173+
assert.ok(result.includes(';'), 'PowerShell should use ; to join commands');
174+
assert.ok(!result.startsWith(' '), 'PowerShell command should not start with a leading space');
175+
});
176+
177+
test('should format multiple commands with "; and" for fish', () => {
178+
const multiCommand: PythonCommandRunConfiguration[] = [
179+
{ executable: 'source', args: ['/path/to/init'] },
180+
{ executable: 'conda', args: ['activate', 'myenv'] },
181+
];
182+
const result = getShellCommandAsString(ShellConstants.FISH, multiCommand);
183+
assert.ok(result.includes('; and'), 'Fish should use "; and" to join commands');
184+
assert.ok(!result.startsWith(' '), 'Fish command should not start with a leading space');
185+
});
186+
});
187+
});
80188
});

0 commit comments

Comments
 (0)