Skip to content
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

Topiary CLI fails when output file doesn't exist #588

Closed
vkleen opened this issue Jul 28, 2023 · 2 comments · Fixed by #583
Closed

Topiary CLI fails when output file doesn't exist #588

vkleen opened this issue Jul 28, 2023 · 2 comments · Fixed by #583
Labels
P1 critical: next release type: bug

Comments

@vkleen
Copy link
Contributor

vkleen commented Jul 28, 2023

The topiary CLI complains about "File not found" when given an output file that doesn't exist yet, using the -o flag.

The root cause here is that topiary tries to call path::canonicalize() on the output file path. This call will fail for paths that don't point to existing files. In Nickel I intend to solve this issue like this. Basically, if the canonicalize() call fails with "File not found" I just hope for the best and continue with the original path as passed by the user.

@ErinvanderVeen
Copy link
Collaborator

I love it when a bug report comes with a proposed solution.

@ErinvanderVeen ErinvanderVeen added the P1 critical: next release label Jul 28, 2023
@Xophmeister
Copy link
Member

Note: #583 -- when it's ready -- will supersede this problem, as -o will no longer be a thing.

Xophmeister added a commit that referenced this issue Aug 18, 2023
* Note about not using infer_subcommands

* --tolerate-parsing-errors doesn't make sense for visualisation

* Separate out input source types so we can create a unified interface

* Fallback to the given output path if canonicalisation fails

Resolves #588

* We're going to need an InputFile, too

* WIP: InputFile type

* Correct blunder regarding --query

Note that clap doesn't support (--foo [--bar] | --quux) groups very
cleanly; it was a bit of a hack to get this to work, with the result
being the error text being a bit off when an illegal combination is
attempted. I've attempted to compensate for this by making the long help
text quite explicit.

Also updated clap, which contains the fix for clap-rs/clap#5022

* Missed change to README

* Add note RE clap-rs/clap#4707 workaround

* Machinery to unify inputs + downstream use to reimplement visualisation

* Don't flatten-away errors

* Don't open the input file until we need to read from it

* Into InputFrom should be from &AtLeastOneInput

* Add logging

* Moar logging!1!!
@Xophmeister Xophmeister mentioned this issue Aug 25, 2023
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 critical: next release type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants