Skip to content

Comments

move ident and remove parse int#37

Merged
hayanesuru merged 4 commits intomainfrom
ident
Feb 14, 2026
Merged

move ident and remove parse int#37
hayanesuru merged 4 commits intomainfrom
ident

Conversation

@hayanesuru
Copy link
Owner

@hayanesuru hayanesuru commented Feb 14, 2026

Summary by CodeRabbit

  • New Features

    • Added Minecraft-style identifier parsing and serialization support.
    • Introduced a length-prefixed byte-array type for network payloads.
  • Refactor

    • Replaced custom boxed-string wrapper with standard library boxed strings.
    • Reduced visibility of several internal array types to streamline the public API.
    • Reorganized workspace modules and moved identifier logic into its own package.
  • Style

    • Improved ergonomic string access for internal string type.

@coderabbitai
Copy link

coderabbitai bot commented Feb 14, 2026

📝 Walkthrough

Walkthrough

Extracts identifier logic into a new haya_ident workspace crate, updates workspace members/dependencies, reduces visibility of several NBT array types, refactors NBT/string parsing to reuse numeric decoding, removes custom integer parsing in haya_ser while adding a ByteArray type, and replaces internal BoxStr with Box<str> across the protocol.

Changes

Cohort / File(s) Summary
Workspace & new crate
Cargo.toml, haya_protocol/Cargo.toml, haya_ident/Cargo.toml, haya_ident/src/lib.rs
Adds haya_ident as a workspace member and dependency; introduces new haya_ident crate providing Ident<'a> parsing/validation/(de)serialization.
Protocol import & API migration
haya_protocol/src/lib.rs, haya_protocol/src/*clientbound*.rs, haya_protocol/src/*serverbound*.rs
Removes local identifier/byte-array types and re-exports; switches imports to use haya_ident::Ident and mser types; updates public structs to rely on external Ident.
String type changes
haya_protocol/src/profile.rs, haya_protocol/src/str.rs
Removes BoxStr and replaces usages with standard Box<str>; deletes BoxStr type and associated impls.
haya_nbt visibility & parsing
haya_nbt/src/byte_array.rs, haya_nbt/src/int_array.rs, haya_nbt/src/long_array.rs, haya_nbt/src/lib.rs, haya_nbt/src/stringify.rs
Makes ByteArray/IntArray/LongArray crate-private and removes their public re-exports; refactors array parsing to use a reusable temporary buffer and unified numeric decoding path.
haya_ser parsing rework
haya_ser/src/integer.rs, haya_ser/src/hex.rs, haya_ser/src/lib.rs
Removes custom integer parsing module; narrows hex parsing macro to unsigned integers (u8–u64); adds a public ByteArray<'a, const MAX> type with Read/Write.
haya_str ergonomics
haya_str/src/lib.rs
Adds Deref and DerefMut implementations for HayaStr to allow direct coercion to str.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • update layout #36: Modifies workspace layout and crate boundaries; likely related to introducing/adjusting haya_ident and haya_str workspace crates.
  • support parse snbt primitive list #34: Adds protocol structs that use Ident<'a>; closely related due to the Ident extraction to haya_ident.

Poem

🐇 I hopped through crates both near and far,

split identifiers like opening a jar.
BoxStr traded for Box, tidy and bright,
bytes and NBT tucked in out of sight.
A rabbit’s refactor — nimble delight!

✨ Finishing touches
  • 📝 Docstrings were successfully generated.
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ident

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
haya_ser/src/hex.rs (1)

44-46: 🧹 Nitpick | 🔵 Trivial

Note: This Integer trait is distinct from the one in integer.rs.

The Integer trait here returns (Self, usize) while the one in integer.rs returns (Self, &[u8]). This is fine as they serve different purposes (hex vs decimal parsing), but the naming overlap could cause confusion for maintainers.

Consider renaming to HexInteger or similar to distinguish it from the decimal parsing trait in integer.rs.

🤖 Fix all issues with AI agents
In `@haya_ident/src/lib.rs`:
- Around line 120-163: Add Debug and Hash implementations for the Identifier and
Inner types so Identifier can be logged and used in hashed collections: update
the declarations of Identifier and Inner to derive Debug and Hash (i.e.,
#[derive(Clone, PartialEq, Eq, Debug, Hash)]) and ensure any contained types
(HayaStr and Box<str>) implement Debug and Hash or adapt the impls accordingly;
no changes to Identifier::new, path(), or namespace() logic are needed.

In `@haya_nbt/src/stringify.rs`:
- Around line 58-59: In dec_arr_peek remove the redundant tmp.clear() call at
the start of the function; dec_num already clears tmp before use, so delete the
tmp.clear() in dec_arr_peek to avoid unnecessary work and unintended clearing
when the match falls through to the error case (refer to functions dec_arr_peek
and dec_num to locate the change).

In `@haya_ser/src/lib.rs`:
- Around line 74-103: The Write impl for ByteArray currently casts self.0.len()
to u32 without validating constraints; add a defensive guard in ByteArray::write
that computes let len = self.0.len(); if len > MAX || len > u32::MAX as usize {
panic!("ByteArray length exceeds MAX or u32::MAX"); } then use V21(len as
u32).write(w) and w.write(self.0); also add the same validation at the start of
len_s() (compute len, validate against MAX and u32::MAX, then return V21(len as
u32).len_s() + len) so serialization enforces the same bounds as Read and avoids
silent truncation.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
haya_nbt/src/stringify.rs (1)

58-143: ⚠️ Potential issue | 🟡 Minor

Add tests documenting byte/long array parsing behavior with unsuffixed decimals.

dec_num returns TagPrimitive::Int for unsuffixed decimal literals, which means [B;1] and [L;1] will fail to parse (the pattern match in dec_arr_peek expects TagPrimitive::Byte and TagPrimitive::Long respectively). This is intentional per the SNBT format, but the behavior lacks test coverage. Add test cases demonstrating that unsuffixed decimals require explicit suffixes in byte and long arrays, or update the parser to coerce Int to Byte/Long if backwards compatibility is needed.

🤖 Fix all issues with AI agents
In `@haya_ident/src/lib.rs`:
- Around line 26-68: The parser currently accepts empty paths (e.g.,
"minecraft:" or ""), so update parse_ident_ascii (and code paths in parse_ident
that call it) to reject empty path segments: before returning Some(...,
from_utf8_unchecked(path)) ensure path is non-empty (path.len() > 0) in the
"minecraft:" branch, in the split_once branch (the path from split_once) and in
the final branch where ident is treated as the path; keep the existing
is_valid_path checks but add the non-empty check so empty slices are treated as
invalid.

@coderabbitai
Copy link

coderabbitai bot commented Feb 14, 2026

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #38

coderabbitai bot added a commit that referenced this pull request Feb 14, 2026
Docstrings generation was requested by @hayanesuru.

* #37 (comment)

The following files were modified:

* `haya_ident/src/lib.rs`
* `haya_nbt/src/byte_array.rs`
* `haya_nbt/src/int_array.rs`
* `haya_nbt/src/long_array.rs`
* `haya_nbt/src/stringify.rs`
* `haya_protocol/src/lib.rs`
* `haya_ser/src/hex.rs`
* `haya_ser/src/lib.rs`
* `haya_str/src/lib.rs`
@hayanesuru hayanesuru merged commit 90e41cb into main Feb 14, 2026
2 checks passed
@hayanesuru hayanesuru deleted the ident branch February 14, 2026 15:20
This was referenced Feb 18, 2026
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.

1 participant