From f772a88a4cfe616c87a4b461660e2cdaa280077a Mon Sep 17 00:00:00 2001 From: Vladimir Lagunov Date: Tue, 5 Dec 2023 15:35:55 +0100 Subject: [PATCH] IJent refactoring: make IjentExecApi.executeProcess sustainable to API changes and backward compatibility Methods with lots of default arguments can cause problems with keeping backward compatibility with different plugins, internal and external. The builder pattern allows to add new optional arguments easier. GitOrigin-RevId: 28291af90d9b65aaeece12cd5b5a64541f73435b --- .../ijent/AbstractIjentVerificationAction.kt | 1 + .../intellij/platform/ijent/IjentExecApi.kt | 44 ++++++++++++++----- .../intellij/execution/wsl/WslIjentManager.kt | 16 +++---- 3 files changed, 39 insertions(+), 22 deletions(-) diff --git a/platform/execution-impl/src/com/intellij/execution/wsl/ijent/AbstractIjentVerificationAction.kt b/platform/execution-impl/src/com/intellij/execution/wsl/ijent/AbstractIjentVerificationAction.kt index 903efafac7f1..3a5a0f9ecb30 100644 --- a/platform/execution-impl/src/com/intellij/execution/wsl/ijent/AbstractIjentVerificationAction.kt +++ b/platform/execution-impl/src/com/intellij/execution/wsl/ijent/AbstractIjentVerificationAction.kt @@ -19,6 +19,7 @@ import com.intellij.platform.ide.progress.withModalProgress import com.intellij.platform.ijent.IjentApi import com.intellij.platform.ijent.IjentExecApi import com.intellij.platform.ijent.IjentMissingBinary +import com.intellij.platform.ijent.executeProcess import com.intellij.platform.ijent.fs.nio.asNioFileSystem import com.intellij.platform.util.coroutines.childScope import kotlinx.coroutines.* diff --git a/platform/ijent/src/com/intellij/platform/ijent/IjentExecApi.kt b/platform/ijent/src/com/intellij/platform/ijent/IjentExecApi.kt index 9348f33198da..942ee2c14d6d 100644 --- a/platform/ijent/src/com/intellij/platform/ijent/IjentExecApi.kt +++ b/platform/ijent/src/com/intellij/platform/ijent/IjentExecApi.kt @@ -1,6 +1,7 @@ // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.platform.ijent +import com.intellij.util.SmartList import org.jetbrains.annotations.ApiStatus /** @@ -14,22 +15,28 @@ interface IjentExecApi { * Starts a process on a remote machine. Right now, the child process may outlive the instance of IJent. * stdin, stdout and stderr of the process are always forwarded, if there are. * - * Beware that processes with [pty] usually don't have stderr. The [IjentChildProcess.stderr] must be an empty stream in such case. + * Beware that processes with [ExecuteProcessArgs.pty] usually don't have stderr. + * The [IjentChildProcess.stderr] must be an empty stream in such case. * - * By default, environment is always inherited from the running IJent instance, which may be unwanted. [env] allows to alter - * some environment variables, it doesn't clear the variables from the parent. When the process should be started in an environment like - * in a terminal, the response of [fetchLoginShellEnvVariables] should be put into [env]. + * By default, environment is always inherited from the running IJent instance, which may be unwanted. [ExecuteProcessArgs.env] allows + * to alter some environment variables, it doesn't clear the variables from the parent. When the process should be started in an + * environment like in a terminal, the response of [fetchLoginShellEnvVariables] should be put into [ExecuteProcessArgs.env]. * * All argument, all paths, should be valid for the remote machine. F.i., if the IDE runs on Windows, but IJent runs on Linux, - * [workingDirectory] is the path on the Linux host. There's no automatic path mapping in this interface. + * [ExecuteProcessArgs.workingDirectory] is the path on the Linux host. There's no automatic path mapping in this interface. */ - suspend fun executeProcess( - exe: String, - vararg args: String, - env: Map = emptyMap(), - pty: Pty? = null, - workingDirectory: String? = null, - ): ExecuteProcessResult + suspend fun executeProcess(args: ExecuteProcessArgs): ExecuteProcessResult + + /** Docs: [executeProcess] */ + class ExecuteProcessArgs(var exe: String) { + var args: MutableList = SmartList() + var env: MutableMap = HashMap(0) + var pty: Pty? = null + var workingDirectory: String? = null + + override fun toString(): String = + "ExecuteProcessArgs(exe='$exe', args=$args, env=$env, pty=$pty, workingDirectory=$workingDirectory)" + } /** * Gets the same environment variables on the remote machine as the user would get if they run the shell. @@ -44,3 +51,16 @@ interface IjentExecApi { /** [echo] must be true in general and must be false when the user is asked for a password. */ data class Pty(val columns: Int, val rows: Int, val echo: Boolean) } + +/** Docs: [IjentExecApi.executeProcess] */ +suspend inline fun IjentExecApi.executeProcess( + exe: String, + vararg args: String, + builder: IjentExecApi.ExecuteProcessArgs.() -> Unit = {}, +): IjentExecApi.ExecuteProcessResult { + require(exe.isNotEmpty()) { "Executable must be specified" } + return executeProcess(IjentExecApi.ExecuteProcessArgs(exe).apply { + this.args += args + builder() + }) +} \ No newline at end of file diff --git a/platform/platform-impl/src/com/intellij/execution/wsl/WslIjentManager.kt b/platform/platform-impl/src/com/intellij/execution/wsl/WslIjentManager.kt index 164a9540d5d4..661fe01b95f1 100644 --- a/platform/platform-impl/src/com/intellij/execution/wsl/WslIjentManager.kt +++ b/platform/platform-impl/src/com/intellij/execution/wsl/WslIjentManager.kt @@ -12,10 +12,7 @@ import com.intellij.openapi.project.Project import com.intellij.openapi.util.IntellijInternalApi import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.registry.Registry -import com.intellij.platform.ijent.IjentApi -import com.intellij.platform.ijent.IjentChildProcess -import com.intellij.platform.ijent.IjentExecApi -import com.intellij.platform.ijent.IjentSessionProvider +import com.intellij.platform.ijent.* import com.intellij.util.SuspendingLazy import com.intellij.util.concurrency.annotations.RequiresBackgroundThread import com.intellij.util.concurrency.annotations.RequiresBlockingContext @@ -94,13 +91,12 @@ class WslIjentManager private constructor(private val scope: CoroutineScope) { val command = processBuilder.command() val ijentApi = getIjentApi(wslDistribution, project, isSudo) - when (val processResult = ijentApi.exec.executeProcess( - exe = FileUtil.toSystemIndependentName(command.first()), - args = command.toList().drop(1).toTypedArray(), - env = processBuilder.environment(), - pty = pty, + when (val processResult = ijentApi.exec.executeProcess(FileUtil.toSystemIndependentName(command.first())) { + args += command.toList().drop(1) + env += processBuilder.environment() + this.pty = pty workingDirectory = processBuilder.directory()?.let { wslDistribution.getWslPath(it.toPath()) } - )) { + }) { is IjentExecApi.ExecuteProcessResult.Success -> processResult.process.toProcess(scope, pty != null) is IjentExecApi.ExecuteProcessResult.Failure -> throw IOException(processResult.message) }