Skip to content

Some updates and some minor improvements#43

Merged
nickspring merged 26 commits into
nickspring:mainfrom
KumoCorp:main
Sep 20, 2025
Merged

Some updates and some minor improvements#43
nickspring merged 26 commits into
nickspring:mainfrom
KumoCorp:main

Conversation

@wez

@wez wez commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

Hi! Please let me know if you'd rather see this PR split up. Each commit is small and easy to reason about, although the tip of this PR has some fixup commits for CI.

The majority of the commits are mechanical updates for deps which address one or two open issues.

The changes that need the most (but honestly not much) scrutiny are:

  1. from_bytes() now returns Result so that it can avoid unwrap() induced panic for things like invalid charset names. I'd like to embed this crate in a stateful service, so the panic is highly undesirable.
  2. CharsetMatch uses Cow<'static, [u8]> instead of Vec<u8> to keep the number of payload copies to just 1 per request, rather than 1 per match per request, to keep the memory usage a little smaller.

I enabled GH actions and fixed up the CI in my fork; you may wish to re-enable them in your fork? It didn't look like they were running in your fork?

wez added 26 commits September 19, 2025 10:55
```
warning: hiding a lifetime that's elided elsewhere is confusing
   --> src/entity.rs:353:21
    |
353 |     pub fn iter_mut(&mut self) -> CharsetMatchesIterMut {
    |                     ^^^^^^^^^     --------------------- the same lifetime is hidden here
    |                     |
    |                     the lifetime is elided here
    |
    = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
    = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
    |
353 |     pub fn iter_mut(&mut self) -> CharsetMatchesIterMut<'_> {
    |                                                        ++++

warning: hiding a lifetime that's elided elsewhere is confusing
   --> src/entity.rs:358:17
    |
358 |     pub fn iter(&self) -> CharsetMatchesIter {
    |                 ^^^^^     ------------------ the same lifetime is hidden here
    |                 |
    |                 the lifetime is elided here
    |
    = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
help: use `'_` for type paths
    |
358 |     pub fn iter(&self) -> CharsetMatchesIter<'_> {
    |                                             ++++
```
This reduces the dependency footprint when embedding into another
application.
Note that this crate can be replaced with functionality in the
rust std lib these days, but that would require a newer version
of rust; I don't want to get into that discussion in this
patch series.
remove a trailing whitespace; no functional change
hoist the Name -> String conversion outside of iterator so that
we do it just once per character, rather than once per pattern.
Just make a single copy of the payload per guess, rather than 1 per
candidate match.

Ideally we'd use a lifetime to reference the input payload,
but the submatch stuff makes that difficult to wrangle,
so we satisfy ourselves with a Cow.

Since this commit changes the published API, bump the version
number in the manifest.
My immediate goal is to avoid the unwrap() panic when an invalid
encoding name is passed to the library, as this is highly undesirable in
a highly stateful service application.

This commit replaces some bare `.unwrap()` calls with either error
message propagation or `.expect(REASON)` to indicate why the panic
can never happen at runtime.
This refreshes various deps in the lock file to make `cargo audit`
happier.
Define a total order via OrderedFloat; this allows the replacement
of a somewhat sketchy `partial_cmp().unwrap()` with `cmp()`, reducing
the number of places in this crate that might panic.

Part of this commit makes CoherenceMatches have pub(crate) visibility;
it appears not to part of any public API, and this commit changes
the type of one its fields, so this commit is an API change anyway.
This removes them from the crate API surface area, and makes it
a bit easier to reason about what logic is consuming them.
@nickspring nickspring merged commit 1bcded5 into nickspring:main Sep 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants