Better Message

[AndrewSav]

I would like an overload for Message for both TextParser and TokenListParser that looks like this:

        /// <summary>
        /// Construct a parser that fails with error message <paramref name="messageGenerator"/> when <paramref name="parser"/> fails.
        /// </summary>
        /// <typeparam name="TKind">The kind of the tokens being parsed.</typeparam>
        /// <typeparam name="T">The type of value being parsed.</typeparam>
        /// <param name="parser">The parser.</param>
        /// <param name="messageGenerator">A function that received the message result of the failed parser and returns the error message.</param>
        /// <returns>The resulting parser.</returns>
        public static TokenListParser<TKind, T> Message<TKind, T>(this TokenListParser<TKind, T> parser, Func<TokenListParserResult<TKind,T>, string> messageGenerator)
        {
            if (parser == null) throw new ArgumentNullException(nameof(parser));
            if (messageGenerator == null) throw new ArgumentNullException(nameof(messageGenerator));

            return input =>
            {
                var result = parser(input);
                if (result.HasValue)
                    return result;

                return TokenListParserResult.Empty<TKind, T>(result.Remainder, result.SubTokenErrorPosition, messageGenerator(result));
            };
        }

That would allow me to use information in the result object for constructing the error message. This will be especially useful since the error message presence overrides expectations, and sometimes they are something that I would like to include into the resulting error message.

Will you consider accepting such a PR?

[nblumhardt]

Is the TokenListParserResult<T> argument rich enough to make constructing good error messages possible? Seems reasonable to me, but it would be great to include a description of a realistic example here or in the PR, to make sure everything hangs together end-to-end 👍

[AndrewSav]

Here https://github.com/AndrewSav/CommandLineParserPoc/blob/ca154fec085b96966d5cd45373270bef1b40536c/CommandLineParserPoC/CommandLineParser.cs#L178 I want to do something like this:

TokenListParser<Argument, Unit> value = Superpower.Parse.Not(Token.Matching<Argument>(x => x.ArgumentType == ArgumentType.Value, "value"))
    .Message(x => $"We expected a switch. We got value: {x.Value}. Expected: {FormatExpectations(x.Expectations)}");

Does this make sense?

[nblumhardt]

Happy New Year @AndrewSav :-)

Thinking about this some more - seems reasonable to me 👍

[AndrewSav]

I think this one did not work out, but a different solution might be required depending on how #142 pans out.

I was considering here a scenario with using Parse.Not(something) as explained in the other issue, and it seems that my use case is specific to this particular scenario, and is not generic as I imagined. In particular, the example that I gave above:

TokenListParser<Argument, Unit> value = Superpower.Parse.Not(Token.Matching<Argument>(x => x.ArgumentType == ArgumentType.Value, "value"))
    .Message(x => $"We expected a switch. We got value: {x.Value}. Expected: {FormatExpectations(x.Expectations)}");

does not work with the proposed change, it cannot work: x.Value is undefined, because from the original code snippet:

if (result.HasValue)
  return result;

return TokenListParserResult.Empty<TKind, T>(result.Remainder, result.SubTokenErrorPosition, messageGenerator(result));

it will not get to our error message if it does have a value and returns before that. The intention was to display the value the previous Parse.Not rejected, but it is not available here, and because of this the entire idea is suddenly much less appealing.

As a workaround, the existent functionality probably suffice like this:

TokenListParser<Argument, Unit> value = Superpower.Parse.Not(Token.Matching<Argument>(x => x.ArgumentType == ArgumentType.Value, "value"))
               .Named("switch");

That would produce unexpected argument 'not_a_switch', expected switch.

Now if we only could also solve #142, the entire thing would be sorted out. I will close this for now.

To recap the scenario I'm trying to work out: a token only has 4 ways to be parsed: Binary Switch, Value Switch, List Switch and Value (source that might explain that better), we do not expect Value at certain stages during the parsing, and we want to detect this situation early to give a good error message. Current we are getting the error message "AtEnd" when parser has failed to progress for any of the switches.

[AndrewSav]

@nblumhardt happy, ehm...., Queen's Birthday? ;)