1
0
mirror of https://github.com/emilk/egui.git synced 2026-06-26 14:49:06 -04:00

Style: forbid .zip and .chain (#8188)

The `zip(a, b)` variant produces clearer code imho.

Downside: added dependency on `itertools`
This commit is contained in:
Emil Ernerfeldt
2026-05-22 12:25:34 +02:00
committed by GitHub
parent ac2496318f
commit 27373b06d0
18 changed files with 68 additions and 69 deletions

View File

@@ -1257,6 +1257,7 @@ dependencies = [
"document-features",
"emath",
"epaint",
"itertools 0.14.0",
"log",
"nohash-hasher",
"profiling",
@@ -1363,6 +1364,7 @@ dependencies = [
"ehttp",
"enum-map",
"image",
"itertools 0.14.0",
"jiff",
"log",
"mime_guess2",

View File

@@ -96,6 +96,7 @@ glutin-winit = { version = "0.5.0", default-features = false }
harfrust = "0.7.0"
home = "0.5.9"
image = { version = "0.25.6", default-features = false }
itertools = "0.14.0"
jiff = { version = "0.2.23", default-features = false }
js-sys = "0.3.77"
kittest = { version = "0.4.0" }

View File

@@ -489,7 +489,7 @@ mod test {
} else {
// There will be small rounding errors whenever the alpha is not 0 or 255,
// because we multiply and then unmultiply the alpha.
for (&a, &b) in in_rgba.iter().zip(out_rgba.iter()) {
for (&a, &b) in std::iter::zip(&in_rgba, &out_rgba) {
assert!(a.abs_diff(b) <= 3, "{in_rgba:?} != {out_rgba:?}");
}
}

View File

@@ -336,7 +336,7 @@ mod test {
} else {
// There will be small rounding errors whenever the alpha is not 0 or 255,
// because we multiply and then unmultiply the alpha.
for (&a, &b) in in_rgba.iter().zip(out_rgba.iter()) {
for (&a, &b) in std::iter::zip(&in_rgba, &out_rgba) {
assert!(a.abs_diff(b) <= 3, "{in_rgba:?} != {out_rgba:?}");
}
}

View File

@@ -31,11 +31,12 @@ pub fn primary_touch_pos(
runner: &mut AppRunner,
event: &web_sys::TouchEvent,
) -> Option<(egui::Pos2, web_sys::Touch)> {
let all_touches: Vec<_> = (0..event.touches().length())
.filter_map(|i| event.touches().get(i))
// On touchend we don't get anything in `touches`, but we still get `changed_touches`, so include those:
.chain((0..event.changed_touches().length()).filter_map(|i| event.changed_touches().get(i)))
.collect();
// On touchend we don't get anything in `touches`, but we still get `changed_touches`, so include those:
let all_touches: Vec<_> = std::iter::chain(
(0..event.touches().length()).filter_map(|i| event.touches().get(i)),
(0..event.changed_touches().length()).filter_map(|i| event.changed_touches().get(i)),
)
.collect();
if let Some(primary_touch) = runner.input.primary_touch {
// Is the primary touch is gone?

View File

@@ -365,7 +365,7 @@ impl WebPainter for WebPainterWgpu {
// Submit the commands: both the main buffer and user-defined ones.
render_state
.queue
.submit(user_cmd_bufs.into_iter().chain([encoder.finish()]));
.submit(std::iter::chain(user_cmd_bufs, [encoder.finish()]));
if let Some((frame, capture_buffer)) = frame_and_capture_buffer {
if let Some(capture_buffer) = capture_buffer

View File

@@ -715,7 +715,7 @@ impl Painter {
let start = web_time::Instant::now();
render_state
.queue
.submit(user_cmd_bufs.into_iter().chain([encoded]));
.submit(std::iter::chain(user_cmd_bufs, [encoded]));
vsync_sec += start.elapsed().as_secs_f32();
};

View File

@@ -74,6 +74,7 @@ epaint = { workspace = true, default-features = false }
accesskit.workspace = true
ahash.workspace = true
bitflags.workspace = true
itertools.workspace = true
log.workspace = true
nohash-hasher.workspace = true
profiling.workspace = true

View File

@@ -472,17 +472,15 @@ impl<'a> Popup<'a> {
RectAlign::find_best_align(
#[expect(clippy::iter_on_empty_collections)]
#[expect(clippy::or_fun_call)]
once(self.rect_align).chain(
std::iter::chain(
once(self.rect_align),
self.alternative_aligns
// Need the empty slice so the iters have the same type so we can unwrap_or
.map(|a| a.iter().copied().chain([].iter().copied()))
.unwrap_or(
self.rect_align
.symmetries()
.iter()
.copied()
.chain(RectAlign::MENU_ALIGNS.iter().copied()),
),
.map(|a| std::iter::chain(a.iter().copied(), [].iter().copied()))
.unwrap_or(std::iter::chain(
self.rect_align.symmetries().iter().copied(),
RectAlign::MENU_ALIGNS.iter().copied(),
)),
),
self.ctx.content_rect(),
anchor_rect,

View File

@@ -641,11 +641,7 @@ impl ContextImpl {
}
fn all_viewport_ids(&self) -> ViewportIdSet {
self.viewports
.keys()
.copied()
.chain([ViewportId::ROOT])
.collect()
std::iter::chain(self.viewports.keys().copied(), [ViewportId::ROOT]).collect()
}
/// The current active viewport

View File

@@ -232,20 +232,14 @@ pub(crate) fn interact(
// );
// }
let contains_pointer: IdSet = hits
.contains_pointer
.iter()
.chain(&hits.click)
.chain(&hits.drag)
.map(|w| w.id)
.collect();
let contains_pointer: IdSet =
itertools::chain!(&hits.contains_pointer, &hits.click, &hits.drag)
.map(|w| w.id)
.collect();
let hovered = if clicked.is_some() || dragged.is_some() || long_touched.is_some() {
// If currently clicking or dragging, only that and nothing else is hovered.
clicked
.iter()
.chain(&dragged)
.chain(&long_touched)
itertools::chain!(&clicked, &dragged, &long_touched)
.copied()
.collect()
} else {
@@ -268,7 +262,9 @@ pub(crate) fn interact(
let drag_order = hits.drag.and_then(|w| order(w.id)).unwrap_or(0);
let top_interactive_order = click_order.max(drag_order);
let mut hovered: IdSet = hits.click.iter().chain(&hits.drag).map(|w| w.id).collect();
let mut hovered: IdSet = std::iter::chain(&hits.click, &hits.drag)
.map(|w| w.id)
.collect();
for w in &hits.contains_pointer {
let is_interactive = w.sense.senses_click() || w.sense.senses_drag();

View File

@@ -1232,11 +1232,12 @@ impl Areas {
}
pub fn visible_layer_ids(&self) -> ahash::HashSet<LayerId> {
self.visible_areas_last_frame
.iter()
.copied()
.chain(self.visible_areas_current_frame.iter().copied())
.collect()
std::iter::chain(
&self.visible_areas_last_frame,
&self.visible_areas_current_frame,
)
.copied()
.collect()
}
pub(crate) fn visible_windows(&self) -> impl Iterator<Item = (LayerId, &area::AreaState)> {

View File

@@ -74,6 +74,7 @@ egui = { workspace = true, default-features = false }
ahash.workspace = true
enum-map.workspace = true
itertools.workspace = true
log.workspace = true
profiling.workspace = true

View File

@@ -619,11 +619,8 @@ impl TableState {
// to take up the remainder of the current available width.
// Also handles changing item spacing.
let mut sizing = crate::sizing::Sizing::default();
for ((prev_width, max_used), column) in state
.column_widths
.iter()
.zip(&state.max_used_widths)
.zip(columns)
for (prev_width, max_used, column) in
itertools::izip!(&state.column_widths, &state.max_used_widths, columns)
{
use crate::Size;

View File

@@ -222,7 +222,7 @@ impl crate::TestRenderer for WgpuTestRenderer {
self.render_state
.queue
.submit(user_buffers.into_iter().chain(once(encoder.finish())));
.submit(std::iter::chain(user_buffers, once(encoder.finish())));
self.render_state
.device

View File

@@ -508,12 +508,7 @@ impl FontFace {
let axes = font_data.skrifa.axes();
// Override the default coordinates with ones specified via FontTweak, then the ones specified directly via the
// argument (probably from TextFormat).
let settings = self
.tweak
.coords
.as_ref()
.iter()
.chain(coords.as_ref().iter());
let settings = std::iter::chain(self.tweak.coords.as_ref(), coords.as_ref());
let location = axes.location(settings);
StyledMetrics {

View File

@@ -1,7 +1,7 @@
#![expect(clippy::unwrap_used)] // TODO(emilk): remove unwraps
use std::ops::Range;
use std::sync::Arc;
use std::{iter, ops::Range};
use emath::{Align, GuiRounding as _, NumExt as _, Pos2, Rect, Vec2, pos2, vec2};
@@ -244,11 +244,7 @@ fn layout_shaped_run(
let mut cluster_start_byte: usize = 0;
let mut cluster_glyph_count: usize = 0;
for (info, pos) in glyph_buffer
.glyph_infos()
.iter()
.zip(glyph_buffer.glyph_positions())
{
for (info, pos) in iter::zip(glyph_buffer.glyph_infos(), glyph_buffer.glyph_positions()) {
let glyph_id = skrifa::GlyphId::new(info.glyph_id);
let cluster = info.cluster;
let mut advance_width_px = pos.x_advance as f32 * px_scale;
@@ -525,7 +521,7 @@ fn layout_section(
/// Avoids `Box<dyn Iterator>` and `Vec<&str>` allocation.
enum SplitOrWhole<'a> {
Split(std::str::Split<'a, char>),
Whole(std::iter::Once<&'a str>),
Whole(iter::Once<&'a str>),
}
impl<'a> SplitOrWhole<'a> {
@@ -533,7 +529,7 @@ impl<'a> SplitOrWhole<'a> {
if split {
Self::Split(text.split('\n'))
} else {
Self::Whole(std::iter::once(text))
Self::Whole(iter::once(text))
}
}
}
@@ -1406,11 +1402,7 @@ fn shape_text(
let tweak = font_face.tweak();
// Build shaper with variable font instance if variation coordinates are set.
let variations: Vec<harfrust::Variation> = tweak
.coords
.as_ref()
.iter()
.chain(coords.as_ref().iter())
let variations: Vec<harfrust::Variation> = iter::chain(tweak.coords.as_ref(), coords.as_ref())
.map(|&(tag, value)| harfrust::Variation { tag, value })
.collect();
@@ -1439,6 +1431,7 @@ fn shape_text(
#[cfg(test)]
mod tests {
use std::iter;
use super::{super::*, *};
use crate::text::cursor::CCursor;
@@ -1575,10 +1568,11 @@ mod tests {
&mut fonts,
pixels_per_point,
Arc::new(LayoutJob::single_section(
(0..elided_galley.rows[0].char_count_excluding_newline())
.map(|_| ch)
.chain(std::iter::once('…'))
.collect::<String>(),
iter::chain(
(0..elided_galley.rows[0].char_count_excluding_newline()).map(|_| ch),
iter::once('…'),
)
.collect::<String>(),
TextFormat::default(),
)),
);

View File

@@ -72,6 +72,16 @@ def lint_lines(filepath, lines_in):
f"{filepath}:{line_nr}: write 'TODO(username):' instead"
)
if re.search(r"\.zip\(", line):
errors.append(
f"{filepath}:{line_nr}: use `std::iter::zip` or `itertools::izip!` instead of `.zip(`"
)
if re.search(r"\.chain\(", line):
errors.append(
f"{filepath}:{line_nr}: use `std::iter::chain` or `itertools::chain!` instead of `.chain(`"
)
if (
"(target_os" in line
and filepath.startswith("./crates/egui/")
@@ -105,6 +115,10 @@ def test_lint():
self
}
""",
"for (a, b) in std::iter::zip(xs, ys) {}",
"for (a, b, c) in itertools::izip!(xs, ys, zs) {}",
"for x in std::iter::chain(xs, ys) {}",
"for x in itertools::chain!(xs, ys, zs) {}",
]
should_fail = [
@@ -121,6 +135,8 @@ def test_lint():
self
}
""",
"for (a, b) in xs.iter().zip(ys) {}",
"for x in xs.iter().chain(ys) {}",
]
for code in should_pass: