FuelLabs/sway

refactor(fmt): use `drain_filter` during comment formatting when stabilized

Open

#3,925 opened on Jan 30, 2023

View on GitHub
 (1 comment) (0 reactions) (0 assignees)Rust (61,702 stars) (5,421 forks)batch import
P: lowgood first issue

Description

BTreeMap does not have an equivalent for Range that also removes the yielded elements, however it does have a > drain_filter method.

A more efficient (performance and code-wise) alternative to this function using drain_filter might look like:

formatter.comment_map.drain_filter(|bs, _v| bs.start >= start && bs.end <= end)

Originally posted by @mitchmindtree in https://github.com/FuelLabs/sway/pull/3854#discussion_r1083691334

Currently, drain_filter is a a nightly-only API - I opted to use retain instead to filter out already formatted comments. We can perhaps consider doing this once drain_filter is stabilized.

Contributor guide

Tech stack
rust
Domain
tooling
Issue type
refactor
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
mostly clear
Prerequisites
RustBTreeMap drain filternightly Rust
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
30
Research direction
This issue proposes replacing `retain` with `drain filter` in comment formatting when that API stabilizes. The relevant code is in swayfmt/src/comments.rs lines 59-64, and the original suggestion came from PR #3854. To proceed, monitor Rust nightly for `drain filter` stabilization, then implement the change. No current activity or assignee.
refactor(fmt): use `drain_filter` during comment formatting when stabilized · FuelLabs/sway#3925 | Good First Issue