Skip to content

Conversation

@KJ7LNW
Copy link
Contributor

@KJ7LNW KJ7LNW commented Mar 22, 2025

Context

Fixed an issue where the read_file tool would fail when maxReadFileLine=0, as the code attempted to access a negative line index.

Implementation

Added checks to:

  • Only call readLines when maxReadFileLine > 0
  • Return empty string when maxReadFileLine = 0

Before

image

After

works :)

How to Test

  • Set maxReadFileLine=0 in VS Code settings
  • Use read_file tool on any file
  • Verify it completes successfully instead of throwing an error

Get in Touch

Discord: KJ7LNW


Important

Fixes negative line index error in Cline.ts for read_file tool when maxReadFileLine=0.

  • Behavior:
    • Fixes issue in Cline.ts where read_file tool fails with maxReadFileLine=0 by preventing negative line index access.
    • Adds check to only call readLines if maxReadFileLine > 0.
    • Returns empty string if maxReadFileLine = 0.
  • Code Changes:
    • Modifies Cline.ts to include conditional logic around readLines call and content assignment.
    • Updates logic to handle empty content when no lines are read.

This description was created by Ellipsis for fc82873. It will automatically update as commits are pushed.

When maxReadFileLine setting was set to zero, the code would attempt to read lines with a negative end index
(maxReadFileLine - 1 = -1), causing the readLines function to reject the request.

This change:
- Checks if maxReadFileLine is greater than zero before calling readLines
- Returns an empty string when maxReadFileLine is zero
- Ensures content is only formatted with line numbers when it's not empty

Signed-off-by: Eric Wheeler <[email protected]>
@KJ7LNW KJ7LNW requested review from cte and mrubens as code owners March 22, 2025 23:47
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Mar 22, 2025
@mrubens
Copy link
Collaborator

mrubens commented Mar 23, 2025

Hmmm, what does it even mean to have a max of 0 for this? We might want to have a floor for this setting, like 100 lines.

@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Mar 23, 2025

Hmmm, what does it even mean to have a max of 0 for this? We might want to have a floor for this setting, like 100 lines.

It means read_file only shows declaration content and then it uses start/end_line to load only what it needs for incredibly small context. give it a try, it works quite well

@mrubens
Copy link
Collaborator

mrubens commented Mar 23, 2025

Hmmm, what does it even mean to have a max of 0 for this? We might want to have a floor for this setting, like 100 lines.

It means read_file only shows declaration content and then it uses start/end_line to load only what it needs for incredibly small context. give it a try, it works quite well

Ah interesting, I see what you mean. I wonder if we need to change the copy around the setting then to make it more clear that it’s not a hard max, it’s just the default chunk size. Separate from this PR.

@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Mar 23, 2025

good idea

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@changeset-bot
Copy link

changeset-bot bot commented Mar 23, 2025

⚠️ No Changeset found

Latest commit: b0cb47b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 23, 2025
@mrubens
Copy link
Collaborator

mrubens commented Mar 23, 2025

Merged in #1915. Thanks!

@mrubens mrubens closed this Mar 23, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants