From 3dd9503d1dac275e972fa03098e15bbafe2c01d3 Mon Sep 17 00:00:00 2001 From: Alexey Katsman Date: Fri, 21 Aug 2026 23:08:19 +0200 Subject: [PATCH] PY-91785 hide the read-only tool path field when it has no path to show On targets the tool path cannot be persisted, so the field is disabled and has no browse button. When the tool is not installed there, the dialog still showed an empty disabled field under a warning row that already said everything actionable. With canBeEdited off, exactly one row is now on screen: a spinner while the lookup runs, the "no executable found" warning once it comes back empty, and the path field only once a path was resolved. The field stays when a path was found but its version probe failed, so the inline error keeps a visible component to attach to. The warning now also waits for the first verdict instead of claiming the tool is missing before anything has been looked up. The field validator can no longer bail out on !component.isVisible alone: that guard also covers the unselected environment managers, which are all built up front and hidden, so it now asks whether any row of this section is on screen. Otherwise hiding the field dropped the sticky "executable is not detected" info from DialogPanelValidator and re-enabled the wizard's action button on a target where setup was guaranteed to fail. Also disable the field from construction rather than from the first validation round, so a read-only field is never briefly editable. (cherry picked from commit ea3decb37c54e6dce2f80de43210cf2b68e7609e) IJ-MR-219733 GitOrigin-RevId: a4052c2905dcdd964cf6c71ba6e33e9e475c324f --- .../messages/PyBundle.properties | 1 + .../python/sdk/add/v2/ValidatedPathField.kt | 77 +++++++++++++++---- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/python/pluginResources/messages/PyBundle.properties b/python/pluginResources/messages/PyBundle.properties index 4ab2db6d8e2e..3b3022bd784a 100644 --- a/python/pluginResources/messages/PyBundle.properties +++ b/python/pluginResources/messages/PyBundle.properties @@ -551,6 +551,7 @@ sdk.create.existing.unsupported.python.management.warning=Make sure pip an sdk.create.custom.location=Location: sdk.create.custom.venv.missing.text=No {0} executable found sdk.create.custom.tool.not.detected=No {0} executable found. Make sure {0} is installed and added to PATH. +sdk.create.custom.tool.detecting=Detecting\u2026 sdk.create.custom.inherit.packages=Inherit packages from base interpreter sdk.create.custom.python.path=Python path: sdk.create.custom.target.specific.properties=Target-Specific Properties diff --git a/python/src/com/jetbrains/python/sdk/add/v2/ValidatedPathField.kt b/python/src/com/jetbrains/python/sdk/add/v2/ValidatedPathField.kt index 6b1f66093d8c..cb95c752a6dc 100644 --- a/python/src/com/jetbrains/python/sdk/add/v2/ValidatedPathField.kt +++ b/python/src/com/jetbrains/python/sdk/add/v2/ValidatedPathField.kt @@ -14,6 +14,7 @@ import com.intellij.openapi.observable.properties.ObservableMutableProperty import com.intellij.openapi.observable.properties.ObservableProperty import com.intellij.openapi.observable.util.and import com.intellij.openapi.observable.util.not +import com.intellij.openapi.observable.util.or import com.intellij.openapi.observable.util.transform import com.intellij.openapi.project.DumbAwareAction import com.intellij.openapi.project.ProjectManager @@ -35,10 +36,11 @@ import com.intellij.ui.components.fields.ExtendableTextComponent import com.intellij.ui.dsl.builder.Align import com.intellij.ui.dsl.builder.AlignX import com.intellij.ui.dsl.builder.Panel -import com.intellij.ui.dsl.builder.Row import com.intellij.ui.dsl.builder.components.ValidationType import com.intellij.ui.dsl.builder.components.validationTooltip +import com.intellij.ui.dsl.gridLayout.UnscaledGaps import com.intellij.util.asDisposable +import com.intellij.util.ui.AsyncProcessIcon import com.jetbrains.python.PyBundle.message import com.jetbrains.python.onSuccess import kotlinx.coroutines.CoroutineScope @@ -147,6 +149,7 @@ internal class ValidatedPathField>( init { setButtonVisible(canBeEdited) + isEnabled = canBeEdited addDocumentListener(object : DocumentAdapter() { override fun textChanged(e: DocumentEvent) { textInputFlow.value = text @@ -289,26 +292,35 @@ internal class ValidatedPathField>( } } +/** + * Shows [missingExecutableText] in place of the tool path field. Returns the tooltip component, so that the + * caller can tell "hidden because this whole section is not selected" from "hidden because there is nothing + * to show" - see the validation guard in [validatablePathField]. + */ private fun > Panel.missingToolRow( fileSystem: FileSystem<*>, missingExecutableText: @Nls String, installAction: ActionLink?, validatedPathField: ValidatedPathField, -): Row { + visiblePredicate: ObservableProperty, +): JPanel { val selectExecutableLink = if (fileSystem.isBrowsable && fileSystem.toolPathCanBePersisted) ActionLink(message("sdk.create.custom.select.executable.link")) { validatedPathField.button.doClick() } else null - return row("") { - validationTooltip(missingExecutableText, - installAction, - selectExecutableLink, - validationType = ValidationType.WARNING, - inline = true) + lateinit var tooltip: JPanel + row("") { + tooltip = validationTooltip(missingExecutableText, + installAction, + selectExecutableLink, + validationType = ValidationType.WARNING, + inline = true) .align(Align.FILL) .component - } + }.visibleIf(visiblePredicate) + + return tooltip } internal fun > Panel.validatablePathField( @@ -331,14 +343,36 @@ internal fun > Panel.validatablePath canBeEdited = canBeEdited, ) - if (missingExecutableText != null) { + /** A lookup has produced a verdict, as opposed to "nothing has been looked up yet". */ + val hasVerdict = pathValidator.backProperty.transform { it != null } + + /** No path resolved: either nothing was found, or nothing has been looked up yet. */ + val toolMissing = pathValidator.backProperty.transform { it?.pathHolder == null } + + val missingToolTooltip = if (missingExecutableText != null) { missingToolRow( fileSystem = fileSystem, missingExecutableText = missingExecutableText, installAction = if (canBeEdited) installAction else null, - validatedPathField = validatedPathField - ).visibleIf(pathValidator.backProperty.transform { it?.pathHolder == null }.and(pathValidator.isDirtyValue.not())) + validatedPathField = validatedPathField, + // Only claim that the tool is missing once we have actually looked for it. + visiblePredicate = hasVerdict.and(toolMissing).and(pathValidator.isDirtyValue.not()), + ) } + else null + + // The path field is hidden while the tool is being looked up (see below), and on a target that lookup is a + // remote probe. The existing environment selector also hides its interpreter combo until the tool validates, + // so without this row the whole section would render empty for the duration of the probe. + val detectingIcon = if (!canBeEdited) { + AsyncProcessIcon("$labelText detecting").also { icon -> + row(labelText) { + cell(icon).customize(UnscaledGaps(0)) + label(message("sdk.create.custom.tool.detecting")) + }.visibleIf(hasVerdict.not() or pathValidator.isDirtyValue) + } + } + else null val initialValidationRequestor = (validationRequestor and WHEN_PROPERTY_CHANGED(pathValidator.isDirtyValue) @@ -349,12 +383,19 @@ internal fun > Panel.validatablePath } else initialValidationRequestor - row(labelText) { + val fieldRow = row(labelText) { cell(validatedPathField) .align(AlignX.FILL) .validationRequestor(finalValidationRequestor) .validationOnInput { component -> - if (!component.isVisible) return@validationOnInput null + // This section is built for every environment manager and the unselected ones are hidden with + // `rowsRange { }.visibleIf(..)`, so an invisible field usually means "another manager is selected" and + // must not gate the dialog. But the field is also hidden when it has no path to show, and then the + // detecting row or the missing tool warning stands in its place - keep validating in that case, or a + // target without the tool installed would silently enable the action button. + if (!component.isVisible && detectingIcon?.isVisible != true && missingToolTooltip?.isVisible != true) { + return@validationOnInput null + } val isVenvOverridden = when (venvExistenceValidationState?.get()) { is VenvExistenceValidationState.Warning -> true @@ -380,5 +421,13 @@ internal fun > Panel.validatablePath } } + // A path field the user cannot edit and that has no path in it carries no information: there is no browse + // button to pick another executable, and the missing tool row already says that the tool has to be installed + // on the target. Once a path is resolved keep the field even if the version probe failed, so that the error + // stays attached to something visible. + if (!canBeEdited) { + fieldRow.visibleIf(toolMissing.not()) + } + return validatedPathField } \ No newline at end of file