Skip to content

Refactors #520

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.

Sign up for GitHub

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

Merged
tafia merged 6 commits into from
Jul 2, 2025
Merged

Refactors #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
14701fc
style: fix clippy
sftse Oct 27, 2024
1159c86
refactor: better pattern matching
sftse Oct 30, 2024
b131429
fmt
sftse Oct 30, 2024
3be7e45
refactor: remove let _ binding, which confuses into thinking error mi…
sftse Oct 30, 2024
f0c2f35
refactor: remove Option-wrapping of return type, is never None
sftse Oct 30, 2024
1245b88
fix: clippy
sftse Jul 1, 2025
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
10 changes: 5 additions & 5 deletions benches/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn count<R: Reader<BufReader<File>>>(path: &str) -> usize {
let path = format!("{}/{}", env!("CARGO_MANIFEST_DIR"), path);
let mut excel: R = open_workbook(&path).expect("cannot open excel file");

let sheets = excel.sheet_names().to_owned();
let sheets = excel.sheet_names();
let mut count = 0;
for s in sheets {
count += excel
Expand Down Expand Up @@ -50,11 +50,11 @@ fn bench_xlsx_cells_reader(b: &mut Bencher) {
let path = format!("{}/{}", env!("CARGO_MANIFEST_DIR"), path);
let mut excel: Xlsx<_> = open_workbook(&path).expect("cannot open excel file");

let sheets = excel.sheet_names().to_owned();
let sheets = excel.sheet_names();
let mut count = 0;
for s in sheets {
let mut cells_reader = excel.worksheet_cells_reader(&s).unwrap();
while let Some(_) = cells_reader.next_cell().unwrap() {
while cells_reader.next_cell().unwrap().is_some() {
count += 1;
}
}
Expand All @@ -69,11 +69,11 @@ fn bench_xlsb_cells_reader(b: &mut Bencher) {
let path = format!("{}/{}", env!("CARGO_MANIFEST_DIR"), path);
let mut excel: Xlsb<_> = open_workbook(&path).expect("cannot open excel file");

let sheets = excel.sheet_names().to_owned();
let sheets = excel.sheet_names();
let mut count = 0;
for s in sheets {
let mut cells_reader = excel.worksheet_cells_reader(&s).unwrap();
while let Some(_) = cells_reader.next_cell().unwrap() {
while cells_reader.next_cell().unwrap().is_some() {
count += 1;
}
}
Expand Down
10 changes: 5 additions & 5 deletions examples/excel_to_csv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ fn write_range<W: Write>(dest: &mut W, range: &Range<Data>) -> std::io::Result<(
match *c {
Data::Empty => Ok(()),
Data::String(ref s) | Data::DateTimeIso(ref s) | Data::DurationIso(ref s) => {
write!(dest, "{}", s)
write!(dest, "{s}")
}
Data::Float(ref f) => write!(dest, "{}", f),
Data::Float(ref f) => write!(dest, "{f}"),
Data::DateTime(ref d) => write!(dest, "{}", d.as_f64()),
Data::Int(ref i) => write!(dest, "{}", i),
Data::Error(ref e) => write!(dest, "{:?}", e),
Data::Bool(ref b) => write!(dest, "{}", b),
Data::Int(ref i) => write!(dest, "{i}"),
Data::Error(ref e) => write!(dest, "{e:?}"),
Data::Bool(ref b) => write!(dest, "{b}"),
}?;
if i != n {
write!(dest, ";")?;
Expand Down
10 changes: 5 additions & 5 deletions examples/search_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() {
// Search recursively for all excel files matching argument pattern
// Output statistics: nb broken references, nb broken cells etc...
let folder = env::args().nth(1).unwrap_or_else(|| ".".to_string());
let pattern = format!("{}/**/*.xl*", folder);
let pattern = format!("{folder}/**/*.xl*");
let mut filecount = 0;

let mut output = pattern
Expand All @@ -40,14 +40,14 @@ fn main() {
filecount += 1;
match run(f) {
Ok((f, missing, cell_errors)) => {
writeln!(output, "{:?}~{:?}~{}", f, missing, cell_errors)
writeln!(output, "{f:?}~{missing:?}~{cell_errors}")
}
Err(e) => writeln!(output, "{:?}", e),
Err(e) => writeln!(output, "{e:?}"),
}
.unwrap_or_else(|e| println!("{:?}", e))
.unwrap_or_else(|e| println!("{e:?}"))
}

println!("Found {} excel files", filecount);
println!("Found {filecount} excel files");
}

fn run(f: GlobResult) -> Result<(PathBuf, Option<usize>, usize), FileStatus> {
Expand Down
66 changes: 32 additions & 34 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,10 @@ where
///
/// This is implemented only for [`calamine::Xlsx`](crate::Xlsx) and [`calamine::Xlsb`](crate::Xlsb), as Xls and Ods formats
/// do not support lazy iteration.
fn worksheet_range_at_ref(&mut self, n: usize) -> Option<Result<Range<DataRef>, Self::Error>> {
fn worksheet_range_at_ref(
&mut self,
n: usize,
) -> Option<Result<Range<DataRef<'_>>, Self::Error>> {
let name = self.sheet_names().get(n)?.to_string();
Some(self.worksheet_range_ref(&name))
}
Expand Down Expand Up @@ -720,41 +723,36 @@ impl<T: CellType> Range<T> {
/// ```
///
pub fn from_sparse(cells: Vec<Cell<T>>) -> Range<T> {
if cells.is_empty() {
Range::empty()
} else {
// search bounds
let row_start = cells.first().unwrap().pos.0;
let row_end = cells.last().unwrap().pos.0;
let mut col_start = u32::MAX;
let mut col_end = 0;
for c in cells.iter().map(|c| c.pos.1) {
if c < col_start {
col_start = c;
}
if c > col_end {
col_end = c;
}
}
let cols = (col_end - col_start + 1) as usize;
let rows = (row_end - row_start + 1) as usize;
let len = cols.saturating_mul(rows);
let mut v = vec![T::default(); len];
v.shrink_to_fit();
for c in cells {
let row = (c.pos.0 - row_start) as usize;
let col = (c.pos.1 - col_start) as usize;
let idx = row.saturating_mul(cols) + col;
if let Some(v) = v.get_mut(idx) {
*v = c.val;
}
}
Range {
start: (row_start, col_start),
end: (row_end, col_end),
inner: v,
let (row_start, row_end) = match &cells[..] {
[] => return Range::empty(),
[first] => (first.pos.0, first.pos.0),
[first, .., last] => (first.pos.0, last.pos.0),
};
// search bounds
let mut col_start = u32::MAX;
let mut col_end = 0;
for c in cells.iter().map(|c| c.pos.1) {
col_start = min(c, col_start);
col_end = max(c, col_end);
}
let cols = (col_end - col_start + 1) as usize;
let rows = (row_end - row_start + 1) as usize;
let len = cols.saturating_mul(rows);
let mut v = vec![T::default(); len];
v.shrink_to_fit();
for c in cells {
let row = (c.pos.0 - row_start) as usize;
let col = (c.pos.1 - col_start) as usize;
let idx = row.saturating_mul(cols) + col;
if let Some(v) = v.get_mut(idx) {
*v = c.val;
}
}
Range {
start: (row_start, col_start),
end: (row_end, col_end),
inner: v,
}
}

/// Set a value at an absolute position in a `Range`.
Expand Down
2 changes: 1 addition & 1 deletion src/ods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ fn read_pictures<RS: Read + Seek>(
let zname = zfile.name();
// no Thumbnails
if zname.starts_with("Pictures") {
if let Some(ext) = zname.split('.').last() {
if let Some(ext) = zname.split('.').next_back() {
if [
"emf", "wmf", "pict", "jpeg", "jpg", "png", "dib", "gif", "tiff", "eps", "bmp",
"wpg",
Expand Down
18 changes: 7 additions & 11 deletions src/xls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ impl<RS: Read + Seek> Xls<RS> {
}
//0x0201 => cells.push(parse_blank(r.data)?), // 513: Blank
0x0203 => cells.push(parse_number(r.data, &self.formats, self.is_1904)?), // 515: Number
0x0204 => cells.extend(parse_label(r.data, &encoding, biff)?), // 516: Label [MS-XLS 2.4.148]
0x0205 => cells.push(parse_bool_err(r.data)?), // 517: BoolErr
0x0204 => cells.push(parse_label(r.data, &encoding, biff)?), // 516: Label [MS-XLS 2.4.148]
0x0205 => cells.push(parse_bool_err(r.data)?), // 517: BoolErr
0x0207 => {
// 519 String (formula value)
let val = Data::String(parse_string(r.data, &encoding, biff)?);
Expand Down Expand Up @@ -778,7 +778,7 @@ fn parse_short_string(
}

let mut s = String::with_capacity(cch);
let _ = encoding.decode_to(r.data, cch, &mut s, high_byte);
encoding.decode_to(r.data, cch, &mut s, high_byte);
Ok(s)
}

Expand All @@ -799,15 +799,11 @@ fn parse_string(r: &[u8], encoding: &XlsEncoding, biff: Biff) -> Result<String,
};

let mut s = String::with_capacity(cch);
let _ = encoding.decode_to(&r[start..], cch, &mut s, high_byte);
encoding.decode_to(&r[start..], cch, &mut s, high_byte);
Ok(s)
}

fn parse_label(
r: &[u8],
encoding: &XlsEncoding,
biff: Biff,
) -> Result<Option<Cell<Data>>, XlsError> {
fn parse_label(r: &[u8], encoding: &XlsEncoding, biff: Biff) -> Result<Cell<Data>, XlsError> {
if r.len() < 6 {
return Err(XlsError::Len {
typ: "label",
Expand All @@ -818,10 +814,10 @@ fn parse_label(
let row = read_u16(r);
let col = read_u16(&r[2..]);
let _ixfe = read_u16(&r[4..]);
Ok(Some(Cell::new(
Ok(Cell::new(
(row as u32, col as u32),
Data::String(parse_string(&r[6..], encoding, biff)?),
)))
))
}

fn parse_label_sst(r: &[u8], strings: &[String]) -> Result<Option<Cell<Data>>, XlsError> {
Expand Down
2 changes: 1 addition & 1 deletion src/xlsb/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ impl<RS: Read + Seek> Xlsb<RS> {
let mut zfile = self.zip.by_index(i)?;
let zname = zfile.name();
if zname.starts_with("xl/media") {
if let Some(ext) = zname.split('.').last() {
if let Some(ext) = zname.split('.').next_back() {
if [
"emf", "wmf", "pict", "jpeg", "jpg", "png", "dib", "gif", "tiff", "eps",
"bmp", "wpg",
Expand Down
4 changes: 2 additions & 2 deletions src/xlsx/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ impl<RS: Read + Seek> Xlsx<RS> {
let mut zfile = self.zip.by_index(i)?;
let zname = zfile.name();
if zname.starts_with("xl/media") {
if let Some(ext) = zname.split('.').last() {
if let Some(ext) = zname.split('.').next_back() {
if [
"emf", "wmf", "pict", "jpeg", "jpg", "png", "dib", "gif", "tiff", "eps",
"bmp", "wpg",
Expand Down Expand Up @@ -804,7 +804,7 @@ impl<RS: Read + Seek> Xlsx<RS> {
}

/// Get the table by name (ref)
pub fn table_by_name_ref(&mut self, table_name: &str) -> Result<Table<DataRef>, XlsxError> {
pub fn table_by_name_ref(&mut self, table_name: &str) -> Result<Table<DataRef<'_>>, XlsxError> {
let TableMetadata {
name,
sheet_name,
Expand Down
21 changes: 10 additions & 11 deletions tests/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,13 +520,12 @@ fn issue_127() {
.collect();

for ext in &["ods", "xls", "xlsx", "xlsb"] {
let p = format!("{}/tests/issue127.{}", root, ext);
let p = format!("{root}/tests/issue127.{ext}");
let workbook = open_workbook_auto(&p).expect(&p);
assert_eq!(
workbook.sheet_names(),
&ordered_names[..],
"{} sheets should be ordered",
ext
"{ext} sheets should be ordered"
);
}
}
Expand Down Expand Up @@ -714,7 +713,7 @@ fn date_xls() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let duration = chrono::Duration::seconds(255 * 60 * 60 + 10 * 60 + 10);
Expand Down Expand Up @@ -749,7 +748,7 @@ fn date_xls_1904() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let duration = chrono::Duration::seconds(255 * 60 * 60 + 10 * 60 + 10);
Expand Down Expand Up @@ -784,7 +783,7 @@ fn date_xlsx() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let duration = chrono::Duration::seconds(255 * 60 * 60 + 10 * 60 + 10);
Expand Down Expand Up @@ -819,7 +818,7 @@ fn date_xlsx_1904() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let duration = chrono::Duration::seconds(255 * 60 * 60 + 10 * 60 + 10);
Expand Down Expand Up @@ -850,7 +849,7 @@ fn date_xlsx_iso() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));
assert_eq!(range.get_value((0, 0)).unwrap().as_time(), None);
assert_eq!(range.get_value((0, 0)).unwrap().as_datetime(), None);
Expand Down Expand Up @@ -894,7 +893,7 @@ fn date_ods() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let time = chrono::NaiveTime::from_hms_opt(10, 10, 10).unwrap();
Expand Down Expand Up @@ -942,7 +941,7 @@ fn date_xlsb() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let duration = chrono::Duration::seconds(255 * 60 * 60 + 10 * 60 + 10);
Expand Down Expand Up @@ -977,7 +976,7 @@ fn date_xlsb_1904() {

#[cfg(feature = "dates")]
{
let date = chrono::NaiveDate::from_ymd_opt(2021, 01, 01).unwrap();
let date = chrono::NaiveDate::from_ymd_opt(2021, 1, 1).unwrap();
assert_eq!(range.get_value((0, 0)).unwrap().as_date(), Some(date));

let duration = chrono::Duration::seconds(255 * 60 * 60 + 10 * 60 + 10);
Expand Down