[pbs-devel] [RFC proxmox 1/2] schema: print item type-text instead of <array>

[pbs-devel] [RFC proxmox 1/2] schema: print item type-text instead of <array>

[Fabian Grünbichler]

this is only used for CLI synopsis/usage strings, the API viewer already
prints the full type text in a correct format. the old variant was also
rather misleading, since on the CLI we don't pass in an array, but each
item as its own parameter.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
noticed this while working on the pull/sync filtering series, but it
affects quite a lot of stuff, among other things the Updater/Deleteable
CLI, e.g. from `man proxmox-backup-manager`:

       --delete <array>
                     List of properties to delete.

vs.

       --delete disable|validation-delay
                     List of properties to delete.

but some of them might only have <string> as the item type text, which
is not much nicer :-/

the whole "List of .." is confusing anyway, but not easily solvable,
since the description is used for
- API dump/viewer (where it is a list/array of ..)
- usage message/man pages (where it's a parameter that gives a single
  element, but it might be passed in multiple times to construct an
  array)

also for some common occurences, the item description is too generic,
and it's not possible to override the description for external types
with the current api macro.

 proxmox/src/api/format.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox/src/api/format.rs b/proxmox/src/api/format.rs
index 7f16e93..a362b21 100644
--- a/proxmox/src/api/format.rs
+++ b/proxmox/src/api/format.rs
@@ -120,7 +120,7 @@ pub fn get_schema_type_text(schema: &Schema, _style: ParameterDisplayStyle) -> S
             _ => String::from("<number>"),
         },
         Schema::Object(_) => String::from("<object>"),
-        Schema::Array(_) => String::from("<array>"),
+        Schema::Array(schema) => get_schema_type_text(schema.items, _style),
         Schema::AllOf(_) => String::from("<object>"),
     }
 }
-- 
2.30.2

[pbs-devel] [RFC proxmox 2/2] schema: add extra info to array parameters

[Fabian Grünbichler]

it's not immediately obvious that they can be specified more than once
otherwise.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
see previous patch for some remaining warts that are not yet solved..

 proxmox/src/api/format.rs | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/proxmox/src/api/format.rs b/proxmox/src/api/format.rs
index a362b21..64fcd0a 100644
--- a/proxmox/src/api/format.rs
+++ b/proxmox/src/api/format.rs
@@ -134,15 +134,15 @@ pub fn get_property_description(
 ) -> String {
     let type_text = get_schema_type_text(schema, style);
 
-    let (descr, default) = match schema {
-        Schema::Null => ("null", None),
-        Schema::String(ref schema) => (schema.description, schema.default.map(|v| v.to_owned())),
-        Schema::Boolean(ref schema) => (schema.description, schema.default.map(|v| v.to_string())),
-        Schema::Integer(ref schema) => (schema.description, schema.default.map(|v| v.to_string())),
-        Schema::Number(ref schema) => (schema.description, schema.default.map(|v| v.to_string())),
-        Schema::Object(ref schema) => (schema.description, None),
-        Schema::AllOf(ref schema) => (schema.description, None),
-        Schema::Array(ref schema) => (schema.description, None),
+    let (descr, default, extra) = match schema {
+        Schema::Null => ("null", None, None),
+        Schema::String(ref schema) => (schema.description, schema.default.map(|v| v.to_owned()), None),
+        Schema::Boolean(ref schema) => (schema.description, schema.default.map(|v| v.to_string()), None),
+        Schema::Integer(ref schema) => (schema.description, schema.default.map(|v| v.to_string()), None),
+        Schema::Number(ref schema) => (schema.description, schema.default.map(|v| v.to_string()), None),
+        Schema::Object(ref schema) => (schema.description, None, None),
+        Schema::AllOf(ref schema) => (schema.description, None, None),
+        Schema::Array(ref schema) => (schema.description, None, Some(String::from("Can be specified more than once."))),
     };
 
     let default_text = match default {
@@ -150,6 +150,11 @@ pub fn get_property_description(
         None => String::new(),
     };
 
+    let descr = match extra {
+        Some(extra) => format!("{} {}", descr, extra),
+        None => String::from(descr),
+    };
+
     if format == DocumentationFormat::ReST {
         let mut text = match style {
             ParameterDisplayStyle::Config => {
@@ -169,7 +174,7 @@ pub fn get_property_description(
             }
         };
 
-        text.push_str(&wrap_text("", "  ", descr, 80));
+        text.push_str(&wrap_text("", "  ", &descr, 80));
         text.push('\n');
 
         text
@@ -184,7 +189,7 @@ pub fn get_property_description(
         let mut text = format!(" {:-10} {}{}", display_name, type_text, default_text);
         let indent = "             ";
         text.push('\n');
-        text.push_str(&wrap_text(indent, indent, descr, 80));
+        text.push_str(&wrap_text(indent, indent, &descr, 80));
 
         text
     }
-- 
2.30.2

[pbs-devel] applied: [RFC proxmox 1/2] schema: print item type-text instead of <array>

[Thomas Lamprecht]

On 15.09.21 15:36, Fabian Grünbichler wrote:
> this is only used for CLI synopsis/usage strings, the API viewer already
> prints the full type text in a correct format. the old variant was also
> rather misleading, since on the CLI we don't pass in an array, but each
> item as its own parameter.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> noticed this while working on the pull/sync filtering series, but it
> affects quite a lot of stuff, among other things the Updater/Deleteable
> CLI, e.g. from `man proxmox-backup-manager`:
> 
>        --delete <array>
>                      List of properties to delete.
> 
> vs.
> 
>        --delete disable|validation-delay
>                      List of properties to delete.
> 
> but some of them might only have <string> as the item type text, which
> is not much nicer :-/
> 
> the whole "List of .." is confusing anyway, but not easily solvable,
> since the description is used for
> - API dump/viewer (where it is a list/array of ..)
> - usage message/man pages (where it's a parameter that gives a single
>   element, but it might be passed in multiple times to construct an
>   array)
> 
> also for some common occurences, the item description is too generic,
> and it's not possible to override the description for external types
> with the current api macro.
> 
>  proxmox/src/api/format.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks! FYI: I moved most of the diff-stat text to the commit message,
slightly cleaned up. IMO it gives better context.

[pbs-devel] applied: [RFC proxmox 2/2] schema: add extra info to array parameters

[Thomas Lamprecht]

On 15.09.21 15:36, Fabian Grünbichler wrote:
> it's not immediately obvious that they can be specified more than once
> otherwise.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> see previous patch for some remaining warts that are not yet solved..
> 
>  proxmox/src/api/format.rs | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
>

applied, thanks!