Skip to content

Commit

Permalink
Auto merge of #11059 - cassaundra:edit-refactor, r=epage
Browse files Browse the repository at this point in the history
Expose cargo add internals as edit API

Move the manifest editing utilities from cargo add to a new `cargo::util::edit` module as part of prep work for `cargo remove` (#10520). No other substantive changes have been made, as this PR is intended only to reduce the refactoring surface area of the implementation of the feature itself.

## Background

In cargo edit, there are a number of top-level modules which enable editing of Cargo manifest files (including `src/dependency.rs` and `src/manifest.rs`). In #10472, these files were added instead as a submodule of the cargo add command, with the stated intention of breaking them out later for subsequent `cargo-edit` subcommands. This PR follows through on that expectation.

## Decisions

Concerns raised in #10472 regarding this change:

- Where should the editing API should live?
  - Proposal: `cargo::ops::edit`
  - Justification: precedent has been set by `cargo::ops::resolve` and others to have utils shared by multiple ops live in `cargo::ops`. This is also serves to be a rather conservative API change.
  - Concerns: the name `edit` could be overly general for those unfamiliar with the cargo edit project (see alternatives)
  - Alternatives:
    - `cargo::edit`: this seems to me to be too top level, and would confuse users trying to discover the cargo API
    - `cargo::util::edit`: if we want to expose this at a higher level, perhaps renaming to act as a counterpart to `crate::util::toml`
    - For each of these, replace `edit` with `toml_edit`, `toml_mut`, `manifest_edit`, `manifest_mut`, `edit_toml`, `edit_manifest` etc. for a more specific module name
- Any more specific naming of types reduce clashes (e.g. `Dependency` or `Manifest` being fairly generic)
  - Currently the only thing distinguishing these similarly named types is their path, which the `edit` module makes more clear
  - Alternatives: rename to `EditDependency`/`EditManifest`, `TomlDependency`/`TomlManifest`, etc.
  • Loading branch information
bors committed Sep 14, 2022
2 parents 0825039 + d1b041d commit 22dd4bf
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 97 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ use cargo::core::FeatureValue;
use cargo::ops::cargo_add::add;
use cargo::ops::cargo_add::AddOptions;
use cargo::ops::cargo_add::DepOp;
use cargo::ops::cargo_add::DepTable;
use cargo::ops::resolve_ws;
use cargo::util::command_prelude::*;
use cargo::util::interning::InternedString;
use cargo::util::toml_mut::manifest::DepTable;
use cargo::CargoResult;

pub fn cli() -> clap::Command<'static> {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_add/crate_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use anyhow::Context as _;

use super::Dependency;
use super::RegistrySource;
use crate::util::toml_mut::dependency::RegistrySource;
use crate::util::validate_package_name;
use crate::CargoResult;

Expand Down
19 changes: 8 additions & 11 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
//! Core of cargo-add command

mod crate_spec;
mod dependency;
mod manifest;

use std::collections::BTreeMap;
use std::collections::BTreeSet;
Expand All @@ -26,18 +24,17 @@ use crate::core::Registry;
use crate::core::Shell;
use crate::core::Summary;
use crate::core::Workspace;
use crate::util::toml_mut::dependency::Dependency;
use crate::util::toml_mut::dependency::GitSource;
use crate::util::toml_mut::dependency::MaybeWorkspace;
use crate::util::toml_mut::dependency::PathSource;
use crate::util::toml_mut::dependency::Source;
use crate::util::toml_mut::dependency::WorkspaceSource;
use crate::util::toml_mut::manifest::DepTable;
use crate::util::toml_mut::manifest::LocalManifest;
use crate::CargoResult;
use crate::Config;
use crate_spec::CrateSpec;
use dependency::Dependency;
use dependency::GitSource;
use dependency::PathSource;
use dependency::RegistrySource;
use dependency::Source;
use manifest::LocalManifest;

use crate::ops::cargo_add::dependency::{MaybeWorkspace, WorkspaceSource};
pub use manifest::DepTable;

/// Information on what dependencies should be added
#[derive(Clone, Debug)]
Expand Down
1 change: 1 addition & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub mod rustc;
mod semver_ext;
pub mod to_semver;
pub mod toml;
pub mod toml_mut;
mod vcs;
mod workspace;

Expand Down
Loading

0 comments on commit 22dd4bf

Please sign in to comment.