Cargo integration #12
Conversation
| /// | ||
| /// Returns `None` if no ancestor path contains a `Cargo.toml`, or if | ||
| /// the limit of MAX_ANCESTORS ancestors has been reached. | ||
| pub fn find_toml() -> Option<PathBuf> { |
There was a problem hiding this comment.
Any reason to not use cargo read-manifest?
There was a problem hiding this comment.
agh, was trying to find a command like that! thanks for pointing it out, makes my life easier.
i'll change the get_package_name/find_toml functions to go through cargo read-manifest. i think we'll still need the find_target, unless i'm wrong
There was a problem hiding this comment.
I don't think searching for a target dir is 100% reliable, you'll need to account for things like: rust-lang/cargo#1657
I think using cargo as a library might be the most reliable way to ensure things work reliably, you could inspect how cargo read-manifest is written perhaps as a starting point (or another cargo-foo package?).
There was a problem hiding this comment.
fair point. i'll dig into this.
There was a problem hiding this comment.
I'm going to say this PR should be merged as it stands, so users can at least have all the improved ergonomics under default output conditions, which I'm sure covers most people's use cases. We can figure out how to deal with custom output directories as a separate issue.
There was a problem hiding this comment.
The NoTargetDirectory error will require the project to have a target/ directory containing the binary to use cargo-profiler w/o --bin. However, the user still has the option of explicitly providing the path to the binary, if they have a non-standard output directory.
There was a problem hiding this comment.
I think you've made a pragmatic decision 👍
No description provided.