Skip to content

Conversation

@adamsitnik
Copy link
Member

I tried to use the code from the README.md to prepare for a demo I am doing tomorrow and I got NRE. We should be using source rather than processedText and I think it's better to rename processedText to normalizedText to express what it is.

cc @luisquintanilla @tarekgh

Copilot AI review requested due to automatic review settings November 27, 2025 17:10
Copilot finished reviewing on behalf of adamsitnik November 27, 2025 17:14
Copy link
Contributor

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 fixes a null reference exception (NRE) bug in the PACKAGE.md code examples. The examples were attempting to call .Substring() on a potentially null processedText variable. The fix updates the code to use the original source string for substring operations and renames the variable to normalizedText to better reflect its purpose.

Key Changes:

  • Fixed NRE by using source instead of normalizedText in .Substring() calls
  • Renamed processedText to normalizedText for better clarity and API consistency

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

var trimIndex = tokenizer.GetIndexByTokenCountFromEnd(source, 5, out string processedText, out _);
Console.WriteLine($"5 tokens from end: {processedText.Substring(trimIndex)}");
var trimIndex = tokenizer.GetIndexByTokenCountFromEnd(source, 5, out string normalizedText, out _);
Console.WriteLine($"5 tokens from end: {source.Substring(trimIndex)}");
Copy link
Member

Choose a reason for hiding this comment

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

Need to write it as:

Console.WriteLine($"5 tokens from end: {(normalizedText ?? source).Substring(trimIndex)}");

please fix the other on in line 38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants