-
Notifications
You must be signed in to change notification settings - Fork 18
feat: word diffs #33
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
feat: word diffs #33
Changes from all commits
969dde8
a625abc
1bf3ccf
3dba63f
2682999
a175626
fbff98b
ae627f5
acdad54
337bbdb
236ae42
4c14911
83fe6bd
b24e52c
9290066
30d2f41
936bbcf
0194b50
d042243
cd20497
d193772
7724909
e92b3c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,14 @@ pub fn lines(data: &str) -> Lines<'_> { | |||||||||||||||||||||
| Lines(ByteLines(data.as_bytes())) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// Returns a [`TokenSource`] that uses the words in `data` as Tokens. A word is | ||||||||||||||||||||||
| /// a sequence of alphanumeric characters as determined by | ||||||||||||||||||||||
| /// `char::is_alphanumeric`, or a sequence of just the space character ' '. Any | ||||||||||||||||||||||
| /// other characters are their own word. | ||||||||||||||||||||||
|
Comment on lines
+22
to
+23
|
||||||||||||||||||||||
| /// `char::is_alphanumeric`, or a sequence of just the space character ' '. Any | |
| /// other characters are their own word. | |
| /// `char::is_alphanumeric`, or a sequence of one or more consecutive space | |
| /// characters (' '). Any other characters are their own word. |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation mentions "a sequence of alphanumeric characters as determined by char::is_alphanumeric" but the implementation on lines 110-113 also includes underscores (_) as part of alphanumeric words. This is inconsistent with the documentation. Either update the documentation to mention that underscores are included in alphanumeric sequences, or remove the special handling of underscores if they should be treated as separate tokens.
| /// a sequence of alphanumeric characters as determined by | |
| /// `char::is_alphanumeric`, or a sequence of just the space character ' '. Any | |
| /// other characters are their own word. | |
| /// a sequence of "word" characters (those for which `char::is_alphanumeric` | |
| /// returns `true`, plus the underscore character '_'), or a sequence of just | |
| /// the space character ' '. Any other characters are their own word. |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The token estimation heuristic divides string length by 3 (assuming average word length of 3 characters). This could result in poor allocation sizing:
- For typical English text, average word length is closer to 4-5 characters when including spaces and punctuation
- For code with long identifiers, the average could be much higher
- The estimate doesn't account for the fact that each punctuation character becomes its own token
Consider using a more conservative estimate like (self.0.len() / 5) or implementing a sampling approach similar to ByteLines::estimate_tokens() that examines the first portion of the text to calculate the actual average.
| (self.0.len() / 3) as u32 | |
| (self.0.len() / 5) as u32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know about this one, but it seems that heuristics aren't always right and maybe there is a way to make it configurable?
Maybe this word-diff is also so tuned to Latin text that we might say it in the function, i.e. word_diff to latin_word_diff.
Uh oh!
There was an error while loading. Please reload this page.