serokell / coffer

Multi-backend password store with multiple frontends
4 stars 2 forks source link

Use one style for showing parsing errors #98

Closed sancho20021 closed 2 years ago

sancho20021 commented 2 years ago

Clarification and motivation

Currently CLI uses different styles to signal syntax errors.

Examples:

Parser error: 1:11: | 1 | BAD_FORMAT | ^ unexpected end of input expecting ':' or fieldname ...


In case of `sort` custom messages are shown, while in `filter`, `megaparsec`'s error messages appear. To keep things consistent, we can use `megaparsec` for both.

# Acceptance criteria

<!--
Clarify how we can verify that the task is done.
-->
Rewrite parsing of `--sort` option using `megaparsec` instead of `Text.splitOn`.
dcastro commented 2 years ago

So, the output of these 2 commands does indeed seem to be inconsistent:

coffer find --sort BAD_FORMAT
coffer find --filter BAD_FORMAT

And that could be fixed by using megaparsec for parsing the --sort value, instead of Text.splitOn ":".


Regarding the apparent inconsistency with the output of other commands such as coffer create ent --tag BAD~: I would argue that's ok. One could argue that the syntax of that command is valid, it's just that the tag content is invalid. In the first two commands, the syntax of the command was wrong. Therefore, it could be argued that they're different types of errors; therefore, the format of the outputs does not have to match.

sancho20021 commented 2 years ago

Thanks! With this explanation I totally agree. Indeed, using megaparsec for parsing --sort will be good enough. Should I edit the Issue explanation to clarify what exactly we need to change?

dcastro commented 2 years ago

Thanks! With this explanation I totally agree. Indeed, using megaparsec for parsing --sort will be good enough. Should I edit the Issue explanation to clarify what exactly we need to change?

Sure, please add a note with the conclusion of our convo ^^