Skip to content
This repository has been archived by the owner on Aug 15, 2024. It is now read-only.

feat: allow version selection #53

Merged
merged 8 commits into from
Dec 2, 2023
Merged

feat: allow version selection #53

merged 8 commits into from
Dec 2, 2023

Conversation

Rustin170506
Copy link
Owner

close #27

@Rustin170506 Rustin170506 changed the title feat: allow version selection WIP: feat: allow version selection Nov 30, 2023
src/ops/crate_spec.rs Outdated Show resolved Hide resolved
Copy link
Owner Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Self-check

src/bin/cargo-info/command/info.rs Show resolved Hide resolved
tests/testsuite/cargo_information/mod.rs Show resolved Hide resolved
@Rustin170506 Rustin170506 changed the title WIP: feat: allow version selection feat: allow version selection Dec 1, 2023
@Rustin170506 Rustin170506 marked this pull request as ready for review December 1, 2023 03:30
src/ops/info.rs Outdated
@@ -29,12 +33,19 @@ pub fn info(spec: &str, config: &Config, reg_or_index: Option<RegistryOrIndex>)
if let Ok(root) = root_manifest(None, config) {
let ws = Workspace::new(&root, config)?;
if let Some(resolve) = ops::load_pkg_lockfile(&ws)? {
if let Ok(p) = resolve.query(spec) {
if let Ok(p) = resolve.query(spec.name()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You likely want to call specs_to_ids. It can return multiple results, just pick the highest.

  • Avoids the need for the matches call below
  • Picks a value when there is ambiguity, rather than erroring and falling through to the registry (we should have a test case added for this)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little bit confused. It seems the specs_to_ids API also uses the query API internally.

It can only return one matched package or bail out an error:

    /// Checks a list of `PackageId`s to find 1 that matches this `PackageIdSpec`. If 0, 2, or
    /// more are found, then this returns an error.
    pub fn query<I>(&self, i: I) -> CargoResult<PackageId>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overlooked that. We likely want to just call .iter() and do our own calls to matches...

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@Rustin170506 Rustin170506 marked this pull request as draft December 2, 2023 02:47
@Rustin170506 Rustin170506 force-pushed the rustin-patch-spec branch 2 times, most recently from 5e6864e to f661faf Compare December 2, 2023 07:43
Copy link
Owner Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 Self-check

license: unknown
rust-version: unknown
documentation: https://docs.rs/my-package/0.2.3+my-package
note: to see how you depend on my-package, run `cargo tree --package [email protected]+my-package --invert`
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two versions were compared and the highest one was chosen.

@Rustin170506 Rustin170506 marked this pull request as ready for review December 2, 2023 08:03
@epage epage merged commit dfb357d into main Dec 2, 2023
9 checks passed
@epage epage deleted the rustin-patch-spec branch December 2, 2023 14:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow version selection with PackageIdSpec's
2 participants