-
Notifications
You must be signed in to change notification settings - Fork 107
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
Исправление #1471 #1475
Исправление #1471 #1475
Conversation
…и использовании и достижении заданного лимита
WalkthroughThe changes involve modifications to the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/OneScript.StandardLibrary/Net/TCPClient.cs (1)
85-85
: Fix indentation: Replace tab with spacesThe indentation on this line uses a tab character while the rest of the file uses spaces.
- limit -= numberOfBytesRead; + limit -= numberOfBytesRead;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/OneScript.StandardLibrary/Net/TCPClient.cs
(1 hunks)
🔇 Additional comments (1)
src/OneScript.StandardLibrary/Net/TCPClient.cs (1)
84-88
: LGTM! The fix correctly implements the byte limit.
The changes properly ensure that reading stops after the specified number of bytes have been read, fixing the previous behavior where the limit only affected the portion size but didn't stop the reading process.
Let's verify the fix handles all edge cases correctly:
✅ Verification successful
After analyzing the search results, I can now generate the final response:
The fix is correctly scoped and other read implementations are not affected
The investigation of similar read patterns in the codebase shows that:
- The
ReadAllData
implementation in TCPClient is unique in its purpose of reading network data with a byte limit - Other read implementations (DataReader, TextReadImpl, CustomLineFeedStreamReader) handle text-based reading with different semantics and don't share the same byte-limit logic
- The fix is properly contained to the specific use case in TCPClient
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any similar patterns in the codebase that might need the same fix
# Look for other methods that might have similar read-with-limit patterns
ast-grep --pattern 'Read($_, $_, $_)'
# Look for test files that verify this behavior
fd -e cs -e cpp -e c --type f -X rg -l "ReadAllData|read.*limit|limit.*read"
Length of output: 428
Script:
#!/bin/bash
# Let's examine the content of these files to see if they have similar read-with-limit patterns
# that might need the same fix
# Check DataReader.cs for similar patterns
rg -A 5 -B 5 "Read.*limit|limit.*read" ./src/OneScript.StandardLibrary/Binary/DataReader.cs
# Check TextReadImpl.cs for similar patterns
rg -A 5 -B 5 "Read.*limit|limit.*read" ./src/OneScript.StandardLibrary/Text/TextReadImpl.cs
# Check CustomLineFeedStreamReader.cs for similar patterns
rg -A 5 -B 5 "Read.*limit|limit.*read" ./src/OneScript.StandardLibrary/CustomLineFeedStreamReader.cs
# Look for tests specifically related to TCPClient's ReadAllData
rg -A 5 -B 5 "ReadAllData" ./src/OneScript.StandardLibrary/Net/TCPClient.cs
Length of output: 4461
@@ -81,7 +81,11 @@ private MemoryStream ReadAllData(NetworkStream source, int limit) | |||
int numberOfBytesRead = source.Read(readBuffer, 0, portion); | |||
ms.Write(readBuffer, 0, numberOfBytesRead); | |||
if (useLimit) | |||
limit -= numberOfBytesRead; | |||
{ | |||
limit -= numberOfBytesRead; |
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.
Что-то с отступами, лучше везде использовать пробелы, чтобы на разных машинах (где разный размер табуляции) показывалось одинаково
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.
Что-то с отступами, лучше везде использовать пробелы, чтобы на разных машинах (где разный размер табуляции) показывалось одинаково
Уже не первый раз замечаю подобное на коде от меня. Видимо что-то в vs поломалась, гляну
Если при вызове метода был установлен лимит и размер входящих данных был больше лимита, то на вход Read соединения подается буфер нулевого размера
Summary by CodeRabbit