Skip to content

Conversation

@vinaychandra
Copy link

This PR implements the usual Input types on the nightly only ByteStr type. The implementation is close to the Bytes type but parts borrowed from the str implementation.

Copilot AI review requested due to automatic review settings January 5, 2026 04:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for the nightly-only ByteStr type from std::bstr as an input type for chumsky parsers. The implementation follows the same pattern as the existing Bytes implementation, treating ByteStr as a byte-oriented string type with u8 tokens.

  • Adds the bstr feature flag to the nightly feature set
  • Implements all necessary Input traits for &ByteStr (Input, ExactSizeInput, StrInput, SliceInput, ValueInput)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/lib.rs Adds bstr feature to nightly features and includes new bstr module
src/bstr.rs Implements Input trait and related traits for ByteStr, following the pattern from Bytes implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +2
use super::*;
use core::bstr::ByteStr;
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

Consider adding module-level documentation to explain the purpose of this module and when users should use ByteStr as an input type. Additionally, the Input trait documentation in src/input.rs (around lines 25-29) should be updated to include ByteStr in the list of common input types and their implemented traits, similar to how &str and &[T] are documented.

Copilot uses AI. Check for mistakes.
Copy link
Owner

Choose a reason for hiding this comment

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

Nah, listing it is superfluous and the impl will appear in the docs anyway

@zesterer
Copy link
Owner

Thanks for the PR! I think the current implementation is correct, but I think it should instead call out to the existing slice impl. For example:

#[inline(always)]
unsafe fn next_maybe(
    this: &mut Self::Cache,
    cursor: &mut Self::Cursor,
) -> Option<Self::MaybeToken> {
    <&[u8]>::next_maybe(this, cursor)
}

This makes the intent clearer and helps prevent any future divergence of the implementations.

@vinaychandra
Copy link
Author

I tried this but some methods became too hard to parse. For example, SliceInput::slice. I technically could have left those as-is but at that point, leaving half of this in one state and the other half without touches felt wrong.

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