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
This commit is contained in:
Vladimir Lagunov
2023-12-08 16:37:25 +00:00
committed by intellij-monorepo-bot
parent 86551c2fec
commit f772a88a4c
3 changed files with 39 additions and 22 deletions
@@ -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.*
@@ -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<String, String> = emptyMap(),
pty: Pty? = null,
workingDirectory: String? = null,
): ExecuteProcessResult
suspend fun executeProcess(args: ExecuteProcessArgs): ExecuteProcessResult
/** Docs: [executeProcess] */
class ExecuteProcessArgs(var exe: String) {
var args: MutableList<String> = SmartList()
var env: MutableMap<String, String> = 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()
})
}
@@ -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)
}