-
Notifications
You must be signed in to change notification settings - Fork 13
fix(cli): Fix crash in templateflow get when matching one file #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8831981 to
389a442
Compare
|
Rebased onto #143 to fix tests. |
|
Given that the only failing test was unrelated and this was here for 8 months without objection, merging by lazy consensus after CI finishes unless someone wants to have a quick look. |
mgxd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions, looks good as is
| click.echo('\n'.join(f'{match}' for match in api.get(template, **entities))) | ||
| paths = api.get(template, **entities) | ||
| filenames = [str(paths)] if isinstance(paths, Path) else [str(file) for file in paths] | ||
| click.echo('\n'.join(filenames)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps past the reach of this PR - but it would be nice to print out a more helpful message with empty lists, i.e.
| click.echo('\n'.join(filenames)) | |
| if not filenames: | |
| click.echo('No files found.') | |
| return | |
| click.echo('\n'.join(filenames)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One currently useful feature is that the output is filenames. Helpful messages on stderr are fine, but I wouldn't want to change the utility of stdout to bash scripts.
Fine for another PR, but I don't want to scope creep this one.
Currently: