Better error message when an address does not exist #15788
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
…s-error [ci skip-build-wheels]
# Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
…s-error # Conflicts: # src/python/pants/base/exceptions.py # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
There was a problem hiding this comment.
I think this improves the situation.
However, the origin description "CLI arguments" is a little bit risky. From my experience, it was rather confusing when that origin description was used for the --changed-since option, as the unknown things technically didn't come from the CLI arguments then.
Just something to keep in mind, that we should be really sure that the origin is accurate at all times, or we'll send users off in the wrong direction hunting for issues. And there is something worse than no information, and that is wrong information.
| {namespace}. Did you mean one of these target names?\n | ||
| """ | ||
| + bullet_list(f":{name}" for name in known_names) | ||
| ) |
There was a problem hiding this comment.
I prefer the other way to formulate this. That is, simply state the known target names and leave the question of the message.
| {bullet_list(str(t.address) for t in parametrizations.all)}\n\n | ||
| Did you mean to use one of those addresses? | ||
| """ | ||
| ) |
There was a problem hiding this comment.
This is the other way to formulate this, and this way is the one I prefer. More importantly though is to be consistent, either use "did you mean?" or "these are the known things: ...".
Oh, and now I noticed the "did you mean?" question at the end of the list. That is perfectly fine, and one of the issues I initially had in the first comment that the question was posed as "did you mean of these?" before presenting what the options were..
There was a problem hiding this comment.
Ah, and the reason is that I based my comments on what I read in the PR description, which now is out of date with the code... :P
There was a problem hiding this comment.
Based on your feedback, I made things consistent for both error messages. Thanks!
Agreed. That was before #15730. We were lying and hardcoding the description_of_origin as always being "CLI arguments" even when it was not. |
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Part of #14468.
Before:
After:
[ci skip-rust]
[ci skip-build-wheels]