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: address clippy warnings + code cleanup #125

Merged
merged 7 commits into from
Feb 5, 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
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
rust: [1.36.0, stable, beta, nightly]
rust: [1.47.0, stable, beta, nightly]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}
- name: Unpin dependencies except on MSRV
if: matrix.rust != '1.36.0'
if: matrix.rust != '1.47.0'
run: cargo update
- run: cargo build --all-targets
- run: cargo test
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ documentation = "https://docs.rs/unicode-bidi/"
keywords = ["rtl", "unicode", "text", "layout", "bidi"]
readme="README.md"
edition = "2018"
rust-version = "1.36.0"
rust-version = "1.47.0"
categories = [
"no-std",
"encoding",
Expand Down
5 changes: 1 addition & 4 deletions src/char_data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ pub(crate) fn bidi_matched_opening_bracket(c: char) -> Option<BidiMatchedOpening
}

pub fn is_rtl(bidi_class: BidiClass) -> bool {
match bidi_class {
RLE | RLO | RLI => true,
_ => false,
}
matches!(bidi_class, RLE | RLO | RLI)
}

#[cfg(feature = "hardcoded-data")]
Expand Down
4 changes: 2 additions & 2 deletions src/char_data/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub enum BidiClass {
use self::BidiClass::*;

#[cfg(feature = "hardcoded-data")]
pub const bidi_class_table: &'static [(char, char, BidiClass)] = &[
pub const bidi_class_table: &[(char, char, BidiClass)] = &[
('\u{0}', '\u{8}', BN), ('\u{9}', '\u{9}', S), ('\u{a}', '\u{a}', B), ('\u{b}', '\u{b}', S),
('\u{c}', '\u{c}', WS), ('\u{d}', '\u{d}', B), ('\u{e}', '\u{1b}', BN), ('\u{1c}', '\u{1e}', B),
('\u{1f}', '\u{1f}', S), ('\u{20}', '\u{20}', WS), ('\u{21}', '\u{22}', ON), ('\u{23}',
Expand Down Expand Up @@ -516,7 +516,7 @@ pub const bidi_class_table: &'static [(char, char, BidiClass)] = &[
'\u{e01ef}', NSM), ('\u{f0000}', '\u{ffffd}', L), ('\u{100000}', '\u{10fffd}', L)
];

pub const bidi_pairs_table: &'static [(char, char, Option<char>)] = &[
pub const bidi_pairs_table: &[(char, char, Option<char>)] = &[
('\u{28}', '\u{29}', None), ('\u{5b}', '\u{5d}', None), ('\u{7b}', '\u{7d}', None), ('\u{f3a}',
'\u{f3b}', None), ('\u{f3c}', '\u{f3d}', None), ('\u{169b}', '\u{169c}', None), ('\u{2045}',
'\u{2046}', None), ('\u{207d}', '\u{207e}', None), ('\u{208d}', '\u{208e}', None), ('\u{2308}',
Expand Down
7 changes: 3 additions & 4 deletions src/deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,8 @@ pub fn visual_runs(line: Range<usize>, levels: &[Level]) -> Vec<LevelRun> {

// Found the start of a sequence. Now find the end.
let mut seq_end = seq_start + 1;
while seq_end < run_count {
if levels[runs[seq_end].start] < max_level {
break;
}

while seq_end < run_count && levels[runs[seq_end].start] >= max_level {
seq_end += 1;
}

Expand All @@ -83,6 +81,7 @@ pub fn visual_runs(line: Range<usize>, levels: &[Level]) -> Vec<LevelRun> {

seq_start = seq_end;
}

max_level
.lower(1)
.expect("Lowering embedding level below zero");
Expand Down
91 changes: 36 additions & 55 deletions src/explicit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
//!
//! <http://www.unicode.org/reports/tr9/#Explicit_Levels_and_Directions>

use alloc::vec::Vec;

use super::char_data::{
is_rtl,
BidiClass::{self, *},
Expand All @@ -35,54 +33,56 @@ pub fn compute<'a, T: TextSource<'a> + ?Sized>(
assert_eq!(text.len(), original_classes.len());

// <http://www.unicode.org/reports/tr9/#X1>
let mut stack = DirectionalStatusStack::new();
stack.push(para_level, OverrideStatus::Neutral);
let mut stack = vec![Status {
level: para_level,
status: OverrideStatus::Neutral,
}];

let mut overflow_isolate_count = 0u32;
let mut overflow_embedding_count = 0u32;
let mut valid_isolate_count = 0u32;

for (i, len) in text.indices_lengths() {
let last = stack.last().unwrap();

match original_classes[i] {
// Rules X2-X5c
RLE | LRE | RLO | LRO | RLI | LRI | FSI => {
let last_level = stack.last().level;

// <https://www.unicode.org/reports/tr9/#Retaining_Explicit_Formatting_Characters>
levels[i] = last_level;
levels[i] = last.level;

// X5a-X5c: Isolate initiators get the level of the last entry on the stack.
let is_isolate = match original_classes[i] {
RLI | LRI | FSI => true,
_ => false,
};
let is_isolate = matches!(original_classes[i], RLI | LRI | FSI);
if is_isolate {
// Redundant due to "Retaining explicit formatting characters" step.
// levels[i] = last_level;
match stack.last().status {
// levels[i] = last.level;
match last.status {
OverrideStatus::RTL => processing_classes[i] = R,
OverrideStatus::LTR => processing_classes[i] = L,
_ => {}
}
}

let new_level = if is_rtl(original_classes[i]) {
last_level.new_explicit_next_rtl()
last.level.new_explicit_next_rtl()
} else {
last_level.new_explicit_next_ltr()
last.level.new_explicit_next_ltr()
};

if new_level.is_ok() && overflow_isolate_count == 0 && overflow_embedding_count == 0
{
let new_level = new_level.unwrap();
stack.push(
new_level,
match original_classes[i] {

stack.push(Status {
level: new_level,
status: match original_classes[i] {
RLO => OverrideStatus::RTL,
LRO => OverrideStatus::LTR,
RLI | LRI | FSI => OverrideStatus::Isolate,
_ => OverrideStatus::Neutral,
},
);
});

if is_isolate {
valid_isolate_count += 1;
} else {
Expand Down Expand Up @@ -110,21 +110,21 @@ pub fn compute<'a, T: TextSource<'a> + ?Sized>(
overflow_isolate_count -= 1;
} else if valid_isolate_count > 0 {
overflow_embedding_count = 0;
loop {
// Pop everything up to and including the last Isolate status.
match stack.vec.pop() {
None
| Some(Status {
status: OverrideStatus::Isolate,
..
}) => break,
_ => continue,
}
}

while !matches!(
stack.pop(),
None | Some(Status {
status: OverrideStatus::Isolate,
..
})
) {}

valid_isolate_count -= 1;
}
let last = stack.last();

let last = stack.last().unwrap();
levels[i] = last.level;

match last.status {
OverrideStatus::RTL => processing_classes[i] = R,
OverrideStatus::LTR => processing_classes[i] = L,
Expand All @@ -138,11 +138,12 @@ pub fn compute<'a, T: TextSource<'a> + ?Sized>(
// do nothing
} else if overflow_embedding_count > 0 {
overflow_embedding_count -= 1;
} else if stack.last().status != OverrideStatus::Isolate && stack.vec.len() >= 2 {
stack.vec.pop();
} else if last.status != OverrideStatus::Isolate && stack.len() >= 2 {
stack.pop();
}

// <https://www.unicode.org/reports/tr9/#Retaining_Explicit_Formatting_Characters>
levels[i] = stack.last().level;
levels[i] = stack.last().unwrap().level;
// X9 part of retaining explicit formatting characters.
processing_classes[i] = BN;
}
Expand All @@ -153,8 +154,8 @@ pub fn compute<'a, T: TextSource<'a> + ?Sized>(

// <http://www.unicode.org/reports/tr9/#X6>
_ => {
let last = stack.last();
levels[i] = last.level;

// This condition is not in the spec, but I am pretty sure that is a spec bug.
// https://www.unicode.org/L2/L2023/23014-amd-to-uax9.pdf
if original_classes[i] != BN {
Expand Down Expand Up @@ -188,23 +189,3 @@ enum OverrideStatus {
LTR,
Isolate,
}

struct DirectionalStatusStack {
vec: Vec<Status>,
}

impl DirectionalStatusStack {
fn new() -> Self {
DirectionalStatusStack {
vec: Vec::with_capacity(Level::max_explicit_depth() as usize + 2),
}
}

fn push(&mut self, level: Level, status: OverrideStatus) {
self.vec.push(Status { level, status });
}

fn last(&self) -> &Status {
self.vec.last().unwrap()
}
}
45 changes: 20 additions & 25 deletions src/implicit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,22 +224,20 @@ pub fn resolve_weak<'a, T: TextSource<'a> + ?Sized>(

// W7. If the previous strong char was L, change EN to L.
let mut last_strong_is_l = sequence.sos == L;
for run in &sequence.runs {
for i in run.clone() {
match processing_classes[i] {
EN if last_strong_is_l => {
processing_classes[i] = L;
}
L => {
last_strong_is_l = true;
}
R | AL => {
last_strong_is_l = false;
}
// <https://www.unicode.org/reports/tr9/#Retaining_Explicit_Formatting_Characters>
// Already scanning past BN here.
_ => {}
for i in sequence.runs.iter().cloned().flatten() {
match processing_classes[i] {
EN if last_strong_is_l => {
processing_classes[i] = L;
}
L => {
last_strong_is_l = true;
}
R | AL => {
last_strong_is_l = false;
}
// <https://www.unicode.org/reports/tr9/#Retaining_Explicit_Formatting_Characters>
// Already scanning past BN here.
_ => {}
}
}
}
Expand Down Expand Up @@ -308,7 +306,7 @@ pub fn resolve_neutral<'a, D: BidiDataSource, T: TextSource<'a> + ?Sized>(
found_e = true;
} else if class == not_e {
found_not_e = true;
} else if class == BidiClass::EN || class == BidiClass::AN {
} else if matches!(class, BidiClass::EN | BidiClass::AN) {
// > Within this scope, bidirectional types EN and AN are treated as R.
if e == BidiClass::L {
found_not_e = true;
Expand Down Expand Up @@ -337,15 +335,15 @@ pub fn resolve_neutral<'a, D: BidiDataSource, T: TextSource<'a> + ?Sized>(
.iter_backwards_from(pair.start, pair.start_run)
.map(|i| processing_classes[i])
.find(|class| {
*class == BidiClass::L
|| *class == BidiClass::R
|| *class == BidiClass::EN
|| *class == BidiClass::AN
matches!(
class,
BidiClass::L | BidiClass::R | BidiClass::EN | BidiClass::AN
)
})
.unwrap_or(sequence.sos);

// > Within this scope, bidirectional types EN and AN are treated as R.
if previous_strong == BidiClass::EN || previous_strong == BidiClass::AN {
if matches!(previous_strong, BidiClass::EN | BidiClass::AN) {
previous_strong = BidiClass::R;
}

Expand Down Expand Up @@ -578,8 +576,5 @@ pub fn resolve_levels(original_classes: &[BidiClass], levels: &mut [Level]) -> L
/// <http://www.unicode.org/reports/tr9/#NI>
#[allow(non_snake_case)]
fn is_NI(class: BidiClass) -> bool {
match class {
B | S | WS | ON | FSI | LRI | RLI | PDI => true,
_ => false,
}
matches!(class, B | S | WS | ON | FSI | LRI | RLI | PDI)
}
20 changes: 12 additions & 8 deletions src/level.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@
//!
//! <http://www.unicode.org/reports/tr9/#BD2>

use alloc::string::{String, ToString};
use alloc::vec::Vec;
use core::convert::{From, Into};
use core::slice;
use alloc::{
string::{String, ToString},
vec::Vec,
};
use core::{
convert::{From, Into},
slice,
};

use super::char_data::BidiClass;

Expand Down Expand Up @@ -219,11 +223,11 @@ pub fn has_rtl(levels: &[Level]) -> bool {
levels.iter().any(|&lvl| lvl.is_rtl())
}

impl Into<u8> for Level {
impl From<Level> for u8 {
/// Convert to the level number
#[inline]
fn into(self) -> u8 {
self.number()
fn from(val: Level) -> Self {
val.number()
}
}

Expand All @@ -244,7 +248,7 @@ impl<'a> PartialEq<&'a str> for Level {
}

/// Used for matching levels in conformance tests
impl<'a> PartialEq<String> for Level {
impl PartialEq<String> for Level {
#[inline]
fn eq(&self, s: &String) -> bool {
self == &s.as_str()
Expand Down
Loading