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

refactor: replace iter_join with itertools::join #13275

Merged
merged 1 commit into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions src/cargo/core/compiler/future_incompat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::core::{Dependency, PackageId, Workspace};
use crate::sources::source::QueryKind;
use crate::sources::SourceConfigMap;
use crate::util::cache_lock::CacheLockMode;
use crate::util::{iter_join, CargoResult};
use crate::util::CargoResult;
use anyhow::{bail, format_err, Context};
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
Expand Down Expand Up @@ -228,7 +228,7 @@ impl OnDiskReports {
/// Returns an ANSI-styled report
pub fn get_report(&self, id: u32, package: Option<&str>) -> CargoResult<String> {
let report = self.reports.iter().find(|r| r.id == id).ok_or_else(|| {
let available = iter_join(self.reports.iter().map(|r| r.id.to_string()), ", ");
let available = itertools::join(self.reports.iter().map(|r| r.id), ", ");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you preferred the free method over the extension trait method?

Copy link
Member Author

Choose a reason for hiding this comment

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

The interface is similar to iter_join and can be benefit from IntoIterator.
(not applicable to this case though).

format_err!(
"could not find report with ID {}\n\
Available IDs are: {}",
Expand All @@ -250,7 +250,7 @@ impl OnDiskReports {
Available packages are: {}\n
Omit the `--package` flag to display a report for all packages",
package,
iter_join(report.per_package.keys(), ", ")
itertools::join(report.per_package.keys(), ", ")
)
})?
.to_string()
Expand Down Expand Up @@ -353,14 +353,8 @@ fn get_updates(ws: &Workspace<'_>, package_ids: &BTreeSet<PackageId>) -> Option<
.collect();
updated_versions.sort();

let updated_versions = iter_join(
updated_versions
.into_iter()
.map(|version| version.to_string()),
", ",
);

if !updated_versions.is_empty() {
let updated_versions = itertools::join(updated_versions, ", ");
writeln!(
updates,
"{} has the following newer versions available: {}",
Expand Down
9 changes: 6 additions & 3 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ use crate::core::{Feature, PackageId, Target, Verbosity};
use crate::util::errors::{CargoResult, VerboseError};
use crate::util::interning::InternedString;
use crate::util::machine_message::{self, Message};
use crate::util::{add_path_args, internal, iter_join_onto, profile};
use crate::util::{add_path_args, internal, profile};
use cargo_util::{paths, ProcessBuilder, ProcessError};
use cargo_util_schemas::manifest::TomlDebugInfo;
use cargo_util_schemas::manifest::TomlTrimPaths;
Expand Down Expand Up @@ -910,9 +910,12 @@ fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit, cmd: &mut ProcessBuild
/// [`-Zallow-features`]: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#allow-features
fn add_allow_features(cx: &Context<'_, '_>, cmd: &mut ProcessBuilder) {
if let Some(allow) = &cx.bcx.config.cli_unstable().allow_features {
use std::fmt::Write;
let mut arg = String::from("-Zallow-features=");
let _ = iter_join_onto(&mut arg, allow, ",");
cmd.arg(&arg);
for f in allow {
let _ = write!(&mut arg, "{f},");
}
cmd.arg(arg.trim_end_matches(','));
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ use serde::{Deserialize, Serialize};

use crate::core::resolver::ResolveBehavior;
use crate::util::errors::CargoResult;
use crate::util::{indented_lines, iter_join};
use crate::util::indented_lines;
use crate::Config;

pub const SEE_CHANNELS: &str =
Expand Down Expand Up @@ -603,7 +603,7 @@ impl Features {
bail!(
"the feature `{}` is not in the list of allowed features: [{}]",
feature_name,
iter_join(allow, ", "),
itertools::join(allow, ", "),
);
}
}
Expand Down Expand Up @@ -1031,7 +1031,7 @@ impl CliUnstable {
bail!(
"the feature `{}` is not in the list of allowed features: [{}]",
k,
iter_join(allowed, ", ")
itertools::join(allowed, ", ")
);
}
}
Expand Down
27 changes: 0 additions & 27 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::fmt;
use std::path::{Path, PathBuf};
use std::time::Duration;

Expand Down Expand Up @@ -94,32 +93,6 @@ pub fn human_readable_bytes(bytes: u64) -> (f32, &'static str) {
(bytes / 1024_f32.powi(i as i32), UNITS[i])
}

pub fn iter_join_onto<W, I, T>(mut w: W, iter: I, delim: &str) -> fmt::Result
where
W: fmt::Write,
I: IntoIterator<Item = T>,
T: std::fmt::Display,
{
let mut it = iter.into_iter().peekable();
while let Some(n) = it.next() {
write!(w, "{}", n)?;
if it.peek().is_some() {
write!(w, "{}", delim)?;
}
}
Ok(())
}

pub fn iter_join<I, T>(iter: I, delim: &str) -> String
where
I: IntoIterator<Item = T>,
T: std::fmt::Display,
{
let mut s = String::new();
let _ = iter_join_onto(&mut s, iter, delim);
s
}

pub fn indented_lines(text: &str) -> String {
text.lines()
.map(|line| {
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ fn clean_dry_run() {
// Verify it didn't delete anything.
let after = p.build_dir().ls_r();
assert_eq!(before, after);
let expected = cargo::util::iter_join(before.iter().map(|p| p.to_str().unwrap()), "\n");
let expected = itertools::join(before.iter().map(|p| p.to_str().unwrap()), "\n");
eprintln!("{expected}");
// Verify the verbose output.
p.cargo("clean --dry-run -v")
Expand Down