Skip to content

Commit

Permalink
Simplify withInterpreter signature (#1261)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Nov 7, 2024
1 parent 393bc22 commit b54e651
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 31 deletions.
10 changes: 6 additions & 4 deletions packages/ansible-language-server/src/utils/commandRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export class CommandRunner {
stderr: string;
}> {
let executablePath: string;
let command: string | undefined;
let runEnv: NodeJS.ProcessEnv | undefined;
let command: string | undefined = "";
let runEnv: NodeJS.ProcessEnv;
const isEEEnabled = this.settings.executionEnvironment.enabled;
let interpreterPathFromConfig = this.settings.python.interpreterPath;
if (interpreterPathFromConfig.includes("${workspaceFolder}")) {
Expand All @@ -53,20 +53,22 @@ export class CommandRunner {

// prepare command and env for local run
if (!isEEEnabled) {
[command, runEnv] = withInterpreter(
const result = withInterpreter(
executablePath,
args,
interpreterPath,
this.settings.python.activationScript,
);
command = result.command;
runEnv = result.env;
} else {
// prepare command and env for execution environment run
const executionEnvironment = await this.context.executionEnvironment;
command = executionEnvironment.wrapContainerArgs(
`${executable} ${args}`,
mountPaths,
);
runEnv = undefined;
runEnv = process.env;
}
if (command === undefined) {
return { stdout: "", stderr: "" };
Expand Down
6 changes: 3 additions & 3 deletions packages/ansible-language-server/src/utils/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function withInterpreter(
args: string,
interpreterPath: string,
activationScript: string,
): [string, NodeJS.ProcessEnv | undefined] {
): { command: string; env: NodeJS.ProcessEnv } {
let command = `${executable} ${args}`; // base case

const newEnv = Object.assign({}, process.env, {
Expand All @@ -59,7 +59,7 @@ export function withInterpreter(

if (activationScript) {
command = `bash -c 'source ${activationScript} && ${executable} ${args}'`;
return [command, undefined];
return { command: command, env: process.env };
}

if (interpreterPath) {
Expand All @@ -76,7 +76,7 @@ export function withInterpreter(
newEnv["PATH"] = `${pathEntry}:${process.env.PATH}`;
delete newEnv.PYTHONHOME;
}
return [command, newEnv];
return { command: command, env: newEnv };
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,18 @@ describe("withInterpreter", () => {
interpreterPath,
activationScript,
);
expect(actualCommand[0]).to.equal(expectedCommand);
expect(actualCommand.command).to.equal(expectedCommand);

if (expectedEnv) {
const expectedKeys = Object.keys(expectedEnv);

expectedKeys.forEach((key) => {
expect(actualCommand[1]).to.haveOwnProperty(key);
expect(typeof actualCommand[1] === "object");
if (!actualCommand[1] || typeof expectedEnv === "string") {
expect(actualCommand.env).to.haveOwnProperty(key);
expect(typeof actualCommand.env === "object");
if (!actualCommand.env || typeof expectedEnv === "string") {
expect(false);
} else {
expect(actualCommand[1][key]).to.include(expectedEnv[key]);
expect(actualCommand.env[key]).to.include(expectedEnv[key]);
}
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ export async function activate(context: ExtensionContext): Promise<void> {
const pythonInterpreter = extSettings.settings.interpreterPath;

// specify the current python interpreter path in the pip installation
const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
extSettings.settings,
`${pythonInterpreter} -m pip install ansible-creator`,
"--no-input",
Expand All @@ -466,7 +466,7 @@ export async function activate(context: ExtensionContext): Promise<void> {
}
terminal = vscode.window.createTerminal({
name: "Ansible Terminal",
env: runEnv,
env: env,
});
terminal.show();
terminal.sendText(command);
Expand Down Expand Up @@ -652,7 +652,7 @@ export async function activate(context: ExtensionContext): Promise<void> {
const pythonInterpreter = extSettings.settings.interpreterPath;

// specify the current python interpreter path in the pip installation
const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
extSettings.settings,
`${pythonInterpreter} -m pip install ansible-dev-tools`,
"--no-input",
Expand All @@ -678,7 +678,7 @@ export async function activate(context: ExtensionContext): Promise<void> {

try {
const result = execSync(command, {
env: runEnv,
env: env,
}).toString();
commandOutput = result;
outputChannel.append(commandOutput);
Expand Down
8 changes: 4 additions & 4 deletions src/features/contentCreator/createAnsibleCollectionPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ export class CreateAnsibleCollection {
const extSettings = new SettingsManager();
await extSettings.initialize();

const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
extSettings.settings,
ansibleCreatorInitCommand,
"",
Expand All @@ -433,7 +433,7 @@ export class CreateAnsibleCollection {
let commandOutput = "";

// execute ansible-creator command
const ansibleCreatorExecutionResult = await runCommand(command, runEnv);
const ansibleCreatorExecutionResult = await runCommand(command, env);
commandOutput += `------------------------------------ ansible-creator logs ------------------------------------\n`;
commandOutput += ansibleCreatorExecutionResult.output;
const ansibleCreatorCommandPassed = ansibleCreatorExecutionResult.status;
Expand All @@ -442,14 +442,14 @@ export class CreateAnsibleCollection {
// ade command inherits only the verbosity options from ansible-creator command
console.debug("[ade] command: ", adeCommand);

const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
extSettings.settings,
adeCommand,
"",
);

// execute ade command
const adeExecutionResult = await runCommand(command, runEnv);
const adeExecutionResult = await runCommand(command, env);
commandOutput += `\n\n------------------------------- ansible-dev-environment logs --------------------------------\n`;
commandOutput += adeExecutionResult.output;
}
Expand Down
4 changes: 2 additions & 2 deletions src/features/contentCreator/createAnsibleProjectPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ export class CreateAnsibleProject {
const extSettings = new SettingsManager();
await extSettings.initialize();

const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
extSettings.settings,
ansibleCreatorInitCommand,
"",
Expand All @@ -400,7 +400,7 @@ export class CreateAnsibleProject {
let commandOutput = "";

// execute ansible-creator command
const ansibleCreatorExecutionResult = await runCommand(command, runEnv);
const ansibleCreatorExecutionResult = await runCommand(command, env);
commandOutput += `------------------------------------ ansible-creator logs ------------------------------------\n`;
commandOutput += ansibleCreatorExecutionResult.output;
const commandPassed = ansibleCreatorExecutionResult.status;
Expand Down
4 changes: 2 additions & 2 deletions src/features/contentCreator/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ export async function getBinDetail(cmd: string, arg: string) {
const extSettings = new SettingsManager();
await extSettings.initialize();

const [command, runEnv] = withInterpreter(extSettings.settings, cmd, arg);
const { command, env } = withInterpreter(extSettings.settings, cmd, arg);

try {
const result = cp.execSync(command, {
env: runEnv,
env: env,
});
return result;
} catch {
Expand Down
8 changes: 4 additions & 4 deletions src/features/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,14 +153,14 @@ export class AnsiblePlaybookRunProvider {
// replace spaces in file name with escape sequence '\ '
commandLineArgs.push(playbookFsPath.replace(/(\s)/, "\\ "));
const cmdArgs = commandLineArgs.map((arg) => arg).join(" ");
const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
this.extensionSettings.settings,
runExecutable,
cmdArgs,
);

console.debug(`Running command: ${command}`);
this.invokeInTerminal(command, runEnv);
this.invokeInTerminal(command, env);
}

/**
Expand All @@ -185,13 +185,13 @@ export class AnsiblePlaybookRunProvider {

const cmdArgs = commandLineArgs.map((arg) => arg).join(" ");
const runCmdArgs = `run ${cmdArgs}`;
const [command, runEnv] = withInterpreter(
const { command, env } = withInterpreter(
this.extensionSettings.settings,
runExecutable,
runCmdArgs,
);
console.debug(`Running command: ${command}`);
this.invokeInTerminal(command, runEnv);
this.invokeInTerminal(command, env);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/features/utils/commandRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function withInterpreter(
settings: ExtensionSettings,
runExecutable: string,
cmdArgs: string,
): [string, NodeJS.ProcessEnv | undefined] {
): { command: string; env: NodeJS.ProcessEnv } {
let command = `${runExecutable} ${cmdArgs}`; // base case

const newEnv = Object.assign({}, process.env, {
Expand All @@ -29,7 +29,7 @@ export function withInterpreter(
const activationScript = settings.activationScript;
if (activationScript) {
command = `bash -c 'source ${activationScript} && ${runExecutable} ${cmdArgs}'`;
return [command, undefined];
return { command: command, env: process.env };
}

const interpreterPath = settings.interpreterPath;
Expand All @@ -46,5 +46,5 @@ export function withInterpreter(
newEnv["PATH"] = `${pathEntry}:${process.env.PATH}`;
delete newEnv.PYTHONHOME;
}
return [command, newEnv];
return { command: command, env: newEnv } as const;
}

0 comments on commit b54e651

Please sign in to comment.