Skip to content

Conversation

meleu
Copy link
Collaborator

@meleu meleu commented Aug 3, 2025

When a validate or filter function prints to stderr it's not considered a failure. I'm assuming such behavior is unintended, therefore I'm sending this PR.

It also does in lib/bashly/views/argument/validations.gtx a similar thing done in #629, preventing the validate_* function from running twice.

@meleu meleu requested a review from DannyBen August 3, 2025 01:34
@DannyBen
Copy link
Member

DannyBen commented Aug 3, 2025

The fix for the validation running twice here is definitely welcome, good catch.

However, I am not totally sure about the "print to stderr is also considered an error".
The way I see the validation functions, is that there is a simple contract that the developer needs to fulfil:

  1. The function name needs to start with validate_
  2. It prints if there is an error.

The printing is simply a way of returning a string, since such a feature does not exist in bash.
Also take into account that the printed string cannot just be an output of some external program (which may print its errors to stderr), since it must be something that makes sense in bashly scope: "docker must be installed" or "must be a number".

While I dm not totally opposed to capturing stderr from these validation functions as if it was stdout, I think it dilutes the message and may add confusion.

I am on the fence, hoping to either convince you or be convinced otherwise.

@DannyBen DannyBen added this to the 1.3 milestone Aug 3, 2025
@meleu
Copy link
Collaborator Author

meleu commented Aug 3, 2025

[external program may print errors to stderr]

That was also my thought against this solution.

I'm convinced that the bashly user should handle external program error messages and print something meaningful to the end user.

Instead of changing the behavior here, I think we can make the "validation contract" more explicit in the docs. Then I'm going to open a PR to bashly-book instead.

Closing this one.

@meleu meleu closed this Aug 3, 2025
@DannyBen
Copy link
Member

DannyBen commented Aug 3, 2025

Cool.

This is the documentation - bullet 3 in particular.
https://bashly.dev/advanced/validations/#custom-validations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants