Is it possible to print a consistent error message from a TypedValueParser?

[miguelff]

In order to parse a string as a CSS selector, I implemented the builder::TypedValueParser for SelectorParser. This trait's method parse_ref receives a read-only reference to the Command.

    /// Parse the argument value
    ///
    /// When `arg` is `None`, an external subcommand value is being parsed.
    fn parse_ref(
        &self,
        cmd: &crate::Command,
        arg: Option<&crate::Arg>,
        value: &std::ffi::OsStr,
    ) -> Result<Self::Value, crate::Error>;

A thing I'd like to achieve is to error gracefully when a CSS selector parsed from the argument list is not valid.

To do it I cannot resort to some interesting Error factory methods that are internal to the clap crate like invalid_value; and the public with_description method is deprecated in favor of Command::error

The problem I'm having is that Command::Error receives a mutable reference to the command, while the signature of TypedValueParser::parse_ref receives an immutable reference. So I cannot simply map the error like this:

type Selector = String;

#[derive(Clone, Debug)]
struct SelectorParser;

impl clap::builder::TypedValueParser for SelectorParser {
    type Value = Selector;

    fn parse_ref(
        &self,
        cmd: &clap::Command,
        arg: Option<&clap::Arg>,
        value: &std::ffi::OsStr,
    ) -> Result<Self::Value, clap::Error> {
        let inner = StringValueParser::new();
        let val = inner.parse_ref(cmd, arg, value)?;
        Ok(kuchiki::Selectors::compile(&val)
            .map_err(|_| {
                // 💥 
                //.map_err(|_| {
                //          ^^^ `cmd` is a `&` reference, so the data it refers to cannot be borrowed as mutable
                cmd.error(
                    ErrorKind::ValueValidation,
                    format!(
                        "Invalid CSS selector {}",
                        value.to_string_lossy().to_owned()
                    ),
                )
            })?
            .to_string())
    }
}

#[derive(Parser)]
#[clap(author, version, about, long_about = None)]
struct Cli {
    /// A CSS selector matching the elements to process
    #[clap(value_parser=SelectorParser)]
    selector: Selector,
}

What would be a better way to implement a value parser that performs validation of a string as a CSS selector and reports potential parsing errors back to the user?

Thank you so much for your attention.

PS: |
I'm using clap 3.2.17
I found a related issue, but I still find this question pertinent: #4133

[epage]

First, if your type implements FromStr, you don't need to do anything else as the derive delegates to value_parser!. This will wrap the error for you.

Second, you can pass a fn(&str) -> Result<T, E> to the value_parsr attribute because those functions implement TypedValueParser. This will wrap the error for you.

But we should also support people writing their own value parsers and this is a bug hole for it. Can you create an issue? The reason Command::error needs &mut is we need to do some building to get usage. We cheat in the value parsxers and just don't show usage. Neither approach is open for you to follow.

[miguelff]

Thanks for your reply @epage! ✨

the target Selector type is not under my control, it's actually a kukichi::Selector which does not implement the 'FromStr' trait.

I worked it around by cloning Command on the implementation.

  let mut cmd = cmd.clone();
  cmd.error(
      ErrorKind::ValueValidation,
      format!(
          "Invalid CSS selector {}",
          value.to_string_lossy().to_owned()
      ),
  )
  .exit();

Although App (and its alias Command) is Clone I think it might be worth to open an issue? Let me think about how to frame it.

[miguelff]

Also simplified it further by implementing the newtype pattern for Selector and implementing FromStr on it.

use clap::Parser;
use regex::Regex;

pub struct Selector(String);

impl std::str::FromStr for Selector {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        if let Ok(s) = kuchiki::Selectors::compile(s) {
            Ok(Selector(s.to_string()))
        } else {
            Err(format!("Invalid CSS selector {}", s).to_owned())
        }
    }
}

#[derive(Parser)]
#[clap(author, version, about, long_about = None)]
struct Cli {
    /// Pretty-print the serialised output
    #[clap(short, long, action)]
    pretty: bool,
    /// From the elements denoted by the command line selector, select those containing the text matching this regex
    #[clap(short, long, value_parser, value_name = "REGEX")]
    contents: Option<Regex>,
    /// A CSS selector matching the elements to process
    selector: Selector,
}

fn main() {
    Cli::parse();
}

Coloring is more accurate too.

[epage]

You should also be able to do

use clap::Parser;
use regex::Regex;

fn selector_from_str(s: &str) -> Result<kuchiki::Selectors, String> {
    if let Ok(s) = kuchiki::Selectors::compile(s) {
        Ok(Selector(s.to_string()))
    } else {
        Err(format!("Invalid CSS selector {}", s).to_owned())
    }
}

#[derive(Parser)]
#[clap(author, version, about, long_about = None)]
struct Cli {
    /// Pretty-print the serialised output
    #[clap(short, long, action)]
    pretty: bool,
    /// From the elements denoted by the command line selector, select those containing the text matching this regex
    #[clap(short, long, value_parser, value_name = "REGEX")]
    contents: Option<Regex>,
    /// A CSS selector matching the elements to process
    #[clap(value_parser = selector_from_str)]
    selector: kuchiki::Selectors,
}

fn main() {
    Cli::parse();
}

(of course, if you just want the String, you can change the functions/fields return type)