From 7554e65456041ad24eed6514d031c7f7c36f3db0 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Thu, 31 Oct 2024 13:02:50 -0400
Subject: [PATCH] Improvements to workflow parameter validators.

- Allow specifying a min/max for integer and float parameters.
- Swap language from regex to "Regular Expression" and synchronize help with regular expression fields in rule builder.
- Extend regular expression help.
- Improve default message for in_range validator. Probably a reversion in some ways introduced with https://github.com/galaxyproject/galaxy/pull/19027.
---
 client/src/components/Form/FormElement.vue    | 11 ++-
 .../Form/FormElementHelpMarkdown.vue          | 56 ++++++++++++
 client/src/components/Form/FormInputs.vue     |  2 +
 client/src/components/Help/HelpPopover.vue    |  2 +-
 client/src/components/Help/terms.yml          |  5 ++
 .../RuleBuilder/RegularExpressionInput.vue    | 12 ++-
 .../tool_util/parser/parameter_validators.py  |  6 +-
 lib/galaxy/tools/parameters/basic.py          |  3 +-
 lib/galaxy/workflow/modules.py                | 87 ++++++++++++++++---
 9 files changed, 158 insertions(+), 26 deletions(-)
 create mode 100644 client/src/components/Form/FormElementHelpMarkdown.vue

diff --git a/client/src/components/Form/FormElement.vue b/client/src/components/Form/FormElement.vue
index ed81622799e1..aa3507808661 100644
--- a/client/src/components/Form/FormElement.vue
+++ b/client/src/components/Form/FormElement.vue
@@ -27,6 +27,7 @@ import FormSelection from "./Elements/FormSelection.vue";
 import FormTags from "./Elements/FormTags.vue";
 import FormText from "./Elements/FormText.vue";
 import FormUpload from "./Elements/FormUpload.vue";
+import FormElementHelpMarkdown from "./FormElementHelpMarkdown.vue";
 
 interface FormElementProps {
     id?: string;
@@ -35,6 +36,7 @@ interface FormElementProps {
     title?: string;
     refreshOnChange?: boolean;
     help?: string;
+    helpFormat?: string;
     error?: string;
     warning?: string;
     disabled?: boolean;
@@ -63,6 +65,7 @@ const props = withDefaults(defineProps<FormElementProps>(), {
     connectedDisableText: "Add connection to module.",
     connectedEnableIcon: "fa fa-times",
     connectedDisableIcon: "fa fa-arrows-alt-h",
+    helpFormat: "html",
     workflowBuildingMode: false,
 });
 
@@ -337,7 +340,13 @@ function onAlert(value: string | undefined) {
         </div>
 
         <div v-if="showPreview" class="ui-form-preview pt-1 pl-2 mt-1">{{ previewText }}</div>
-        <span v-if="Boolean(helpText)" class="ui-form-info form-text text-muted" v-html="helpText" />
+        <span
+            v-if="Boolean(helpText) && helpFormat != 'markdown'"
+            class="ui-form-info form-text text-muted"
+            v-html="helpText" />
+        <span v-else-if="Boolean(helpText)" class="ui-form-info form-text text-muted"
+            ><FormElementHelpMarkdown :content="helpText"
+        /></span>
     </div>
 </template>
 
diff --git a/client/src/components/Form/FormElementHelpMarkdown.vue b/client/src/components/Form/FormElementHelpMarkdown.vue
new file mode 100644
index 000000000000..e6271c9a2124
--- /dev/null
+++ b/client/src/components/Form/FormElementHelpMarkdown.vue
@@ -0,0 +1,56 @@
+<script setup lang="ts">
+import { computed, onMounted, ref } from "vue";
+
+import { markup } from "@/components/ObjectStore/configurationMarkdown";
+import { getAppRoot } from "@/onload/loadConfig";
+
+import HelpPopover from "@/components/Help/HelpPopover.vue";
+
+const props = defineProps<{
+    content: string;
+}>();
+
+const markdownHtml = computed(() => markup(props.content ?? "", false));
+
+const helpHtml = ref<HTMLDivElement>();
+
+interface InternalTypeReference {
+    element: HTMLElement;
+    term: string;
+}
+
+const internalHelpReferences = ref<InternalTypeReference[]>([]);
+
+function setupPopovers() {
+    internalHelpReferences.value.length = 0;
+    if (helpHtml.value) {
+        const links = helpHtml.value.getElementsByTagName("a");
+        Array.from(links).forEach((link) => {
+            if (link.href.startsWith("gxhelp://")) {
+                const uri = link.href.substr("gxhelp://".length);
+                internalHelpReferences.value.push({ element: link, term: uri });
+                link.href = `${getAppRoot()}help/terms/${uri}`;
+                link.style.color = "inherit";
+                link.style.textDecorationLine = "underline";
+                link.style.textDecorationStyle = "dashed";
+            }
+        });
+    }
+}
+
+onMounted(setupPopovers);
+</script>
+
+<template>
+    <span>
+        <!-- Disable v-html warning because we allow markdown generated HTML
+            in various places in the Galaxy interface. Raw HTML is not allowed
+            here because admin = false in the call to markup.
+        -->
+        <!-- eslint-disable-next-line vue/no-v-html -->
+        <div ref="helpHtml" v-html="markdownHtml" />
+        <span v-for="(value, i) in internalHelpReferences" :key="i">
+            <HelpPopover :target="value.element" :term="value.term" />
+        </span>
+    </span>
+</template>
diff --git a/client/src/components/Form/FormInputs.vue b/client/src/components/Form/FormInputs.vue
index 1421a4b79ea3..fafe1734b478 100644
--- a/client/src/components/Form/FormInputs.vue
+++ b/client/src/components/Form/FormInputs.vue
@@ -11,6 +11,7 @@
                         v-model="input.test_param.value"
                         :type="input.test_param.type"
                         :help="input.test_param.help"
+                        :help-format="input.test_param.help_format"
                         :refresh-on-change="false"
                         :disabled="sustainConditionals"
                         :attributes="input.test_param"
@@ -51,6 +52,7 @@
                 :error="input.error"
                 :warning="input.warning"
                 :help="input.help"
+                :help-format="input.help_format"
                 :refresh-on-change="input.refresh_on_change"
                 :attributes="input.attributes || input"
                 :collapsed-enable-text="collapsedEnableText"
diff --git a/client/src/components/Help/HelpPopover.vue b/client/src/components/Help/HelpPopover.vue
index 11ded1f423de..200f76706a2b 100644
--- a/client/src/components/Help/HelpPopover.vue
+++ b/client/src/components/Help/HelpPopover.vue
@@ -12,7 +12,7 @@ defineProps<Props>();
 </script>
 
 <template>
-    <BPopover :target="target" triggers="hover" placement="bottom">
+    <BPopover v-if="target" :target="target" triggers="hover" placement="bottom">
         <HelpTerm :term="term" />
     </BPopover>
 </template>
diff --git a/client/src/components/Help/terms.yml b/client/src/components/Help/terms.yml
index 0ca9bd36bc72..536d08f3dee1 100644
--- a/client/src/components/Help/terms.yml
+++ b/client/src/components/Help/terms.yml
@@ -27,7 +27,12 @@ unix:
     debug what is wrong with the execution of an application.
 
     More information on stack traces can be found on [Wikipedia](https://en.wikipedia.org/wiki/Stack_trace).
+programming:
+  python:
+    regex: |
+      Regular expressions are patterns used to match character combinations in strings. This input accepts Python-style regular expressions, find more information about these in [this Python for Biologists tutorial](https://pythonforbiologists.com/tutorial/regex.html).
 
+      The website [regex101](https://regex101.com/) is a useful playground to explore regular expressions (be sure to enable "Python" as your flavor) and language models such as ChatGPT can help interactively build up and explain regular expressions from natural language prompts or examples.
 galaxy:
   collections:
     flatList: |
diff --git a/client/src/components/RuleBuilder/RegularExpressionInput.vue b/client/src/components/RuleBuilder/RegularExpressionInput.vue
index 3f7c089adf80..b3788e423abe 100644
--- a/client/src/components/RuleBuilder/RegularExpressionInput.vue
+++ b/client/src/components/RuleBuilder/RegularExpressionInput.vue
@@ -1,7 +1,7 @@
 <template>
     <div>
-        <label v-b-tooltip.hover for="regular_expression" :title="title">{{ label }}</label>
-        <span v-b-popover.html="popoverContent" :title="popoverTitle" class="fa fa-question"></span>
+        <label ref="helpTarget" v-b-tooltip.hover for="regular_expression">{{ label }}</label>
+        <HelpPopover :target="$refs.helpTarget" term="programming.python.regex" />
         <input
             v-b-tooltip.hover.left
             :title="title"
@@ -16,7 +16,10 @@
 <script>
 import _l from "utils/localization";
 
+import HelpPopover from "@/components/Help/HelpPopover.vue";
+
 export default {
+    components: { HelpPopover },
     props: {
         target: {
             required: true,
@@ -32,11 +35,6 @@ export default {
         popoverTitle() {
             return _l("Regular Expressions");
         },
-        popoverContent() {
-            return _l(
-                `Regular expressions are patterns used to match character combinations in strings. This input accepts Python-style regular expressions, find more information about these in <a href="https://pythonforbiologists.com/tutorial/regex.html">this Python for Biologists tutorial</a>.`
-            );
-        },
     },
 };
 </script>
diff --git a/lib/galaxy/tool_util/parser/parameter_validators.py b/lib/galaxy/tool_util/parser/parameter_validators.py
index b8117c63764b..ca539f74ee1d 100644
--- a/lib/galaxy/tool_util/parser/parameter_validators.py
+++ b/lib/galaxy/tool_util/parser/parameter_validators.py
@@ -218,7 +218,9 @@ def default_message(self) -> str:
             op1 = "<"
         if self.exclude_max:
             op2 = "<"
-        range_description_str = f"({self.min} {op1} value {op2} {self.max})"
+        min_str = str(self.min) if self.min is not None else "-infinity"
+        max_str = str(self.max) if self.max is not None else "+infinity"
+        range_description_str = f"({min_str} {op1} value {op2} {max_str})"
         return f"Value ('%s') must {'not ' if self.negate else ''}fulfill {range_description_str}"
 
 
@@ -466,7 +468,7 @@ def default_message(self) -> str:
 ]
 
 
-DiscriminatedAnyValidatorModel = TypeAdapter(AnyValidatorModel)
+DiscriminatedAnyValidatorModel = TypeAdapter(AnyValidatorModel)  # type:ignore[var-annotated]
 
 
 def parse_dict_validators(validator_dicts: List[Dict[str, Any]], trusted: bool) -> List[AnyValidatorModel]:
diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index cdc5ada345c8..e5e8cfae961d 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -179,7 +179,7 @@ class ToolParameter(UsesDictVisibleKeys):
     """
 
     name: str
-    dict_collection_visible_keys = ["name", "argument", "type", "label", "help", "refresh_on_change"]
+    dict_collection_visible_keys = ["name", "argument", "type", "label", "help", "help_format", "refresh_on_change"]
 
     def __init__(self, tool, input_source, context=None):
         input_source = ensure_input_source(input_source)
@@ -193,6 +193,7 @@ def __init__(self, tool, input_source, context=None):
         self.is_dynamic = False
         self.label = input_source.parse_label()
         self.help = input_source.parse_help()
+        self.help_format = input_source.get("help_format") or "html"
         if (sanitizer_elem := input_source.parse_sanitizer_elem()) is not None:
             self.sanitizer = ToolParameterSanitizer.from_element(sanitizer_elem)
         else:
diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py
index c4fe206fa2e7..6f125b4232cc 100644
--- a/lib/galaxy/workflow/modules.py
+++ b/lib/galaxy/workflow/modules.py
@@ -78,7 +78,9 @@
     BooleanToolParameter,
     DataCollectionToolParameter,
     DataToolParameter,
+    FloatToolParameter,
     HiddenToolParameter,
+    IntegerToolParameter,
     parameter_types,
     raw_to_galaxy,
     SelectToolParameter,
@@ -1282,7 +1284,7 @@ def get_inputs(self):
                 specify_multiple = BooleanToolParameter(None, specify_multiple_source)
 
                 add_validators_repeat = Repeat("validators")
-                add_validators_repeat._title = "Add validator to restrict valid input"
+                add_validators_repeat.title = "Add validator to restrict valid input"
                 add_validators_repeat.min = 0
                 add_validators_repeat.max = math.inf
                 add_validators_repeat.inputs = {
@@ -1291,8 +1293,9 @@ def get_inputs(self):
                         {
                             "optional": False,
                             "name": "regex_match",
-                            "label": "Specify regex",
-                            "help": "Provided regex must match input value for input to be valid",
+                            "label": "Specify regular expression",
+                            "help": "Provided [regular expression](gxhelp://programming.python.reg.ex) must match input value for input to be valid",
+                            "help_format": "markdown",
                         },
                     ),
                     "regex_doc": TextToolParameter(
@@ -1301,7 +1304,7 @@ def get_inputs(self):
                             "optional": False,
                             "name": "regex_doc",
                             "label": "Specify a message",
-                            "help": "This message will be shown if the regex does not match the input",
+                            "help": "This message will be shown if the regular does not match the input",
                         },
                     ),
                 }
@@ -1389,6 +1392,45 @@ def get_inputs(self):
                 restrictions_cond.cases = restrictions_cond_cases
                 when_this_type.inputs["restrictions"] = restrictions_cond
 
+            if param_type == "integer":
+                when_this_type.inputs["min"] = IntegerToolParameter(
+                    None,
+                    {
+                        "name": "min",
+                        "optional": True,
+                        "value": parameter_def.get("min", ""),
+                        "label": "Set a minimum value for this input",
+                    },
+                )
+                when_this_type.inputs["max"] = IntegerToolParameter(
+                    None,
+                    {
+                        "name": "max",
+                        "optional": True,
+                        "value": parameter_def.get("max", ""),
+                        "label": "Set a maximum value for this input",
+                    },
+                )
+            if param_type == "float":
+                when_this_type.inputs["min"] = FloatToolParameter(
+                    None,
+                    {
+                        "name": "min",
+                        "optional": True,
+                        "value": parameter_def.get("min", ""),
+                        "label": "Set a minimum value for this input",
+                    },
+                )
+                when_this_type.inputs["max"] = FloatToolParameter(
+                    None,
+                    {
+                        "name": "max",
+                        "optional": True,
+                        "value": parameter_def.get("max", ""),
+                        "label": "Set a maximum value for this input",
+                    },
+                )
+
             cases.append(when_this_type)
 
         parameter_type_cond.cases = cases
@@ -1503,7 +1545,7 @@ def _parameter_def_list_to_options(parameter_value):
             parameter_kwds["options"] = _parameter_def_list_to_options(restriction_values)
             restricted_inputs = True
 
-        if is_text and parameter_def.get("validators"):
+        if parameter_def.get("validators"):
             parameter_kwds["validators"] = parameter_def["validators"]
 
         client_parameter_type = parameter_type
@@ -1614,13 +1656,18 @@ def step_state_to_tool_state(self, state):
             # the form definition can change from Galaxy to Galaxy fairly freely, but the source validators are persisted
             # and need to be consistent - here we convert the persisted/YAML tool definition version to the "tool form" version.
             for i, source_validator in enumerate(source_validators):
-                form_validators.append(
-                    {
-                        "__index__": i,
-                        "regex_doc": source_validator.get("message"),
-                        "regex_match": source_validator.get("expression"),
-                    }
-                )
+                source_type = source_validator["type"]
+                if source_type == "regex":
+                    form_validators.append(
+                        {
+                            "__index__": i,
+                            "regex_doc": source_validator.get("message"),
+                            "regex_match": source_validator.get("expression"),
+                        }
+                    )
+                elif source_type == "in_range":
+                    state["parameter_definition"]["min"] = source_validator.get("min")
+                    state["parameter_definition"]["max"] = source_validator.get("max")
             state["parameter_definition"]["validators"] = form_validators
         state["parameter_definition"]["restrictions"] = {}
         state["parameter_definition"]["restrictions"]["how"] = restrictions_how
@@ -1665,9 +1712,21 @@ def _parse_state_into_dict(self):
                 optional = False
             if "multiple" in parameters_def:
                 rval["multiple"] = parameters_def["multiple"]
+            source_validators = []
+            if "min" in parameters_def or "max" in parameters_def:
+                min = parameters_def.get("min")
+                max = parameters_def.get("max")
+                source_validators.append(
+                    {
+                        "min": min,
+                        "max": max,
+                        "negate": False,
+                        "type": "in_range",
+                    }
+                )
+
             if "validators" in parameters_def:
                 form_validators = parameters_def["validators"]
-                source_validators = []
                 # convert the current tool form structure to the persisted YAML-definition style
                 for form_validator in form_validators:
                     source_validators.append(
@@ -1678,7 +1737,7 @@ def _parse_state_into_dict(self):
                             "type": "regex",
                         }
                     )
-                rval["validators"] = source_validators
+            rval["validators"] = source_validators
             restrictions_cond_values = parameters_def.get("restrictions")
             if restrictions_cond_values: