Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Apr 2, 2025

#2227


Important

Change line number output to 1-based in processCaptures() and update tests accordingly.

  • Behavior:
    • Change line number output to 1-based in processCaptures() in index.ts.
    • Update test cases in index.test.ts and markdownIntegration.test.ts to expect 1-based line numbers.
  • Tests:
    • Modify expected line numbers in index.test.ts and markdownIntegration.test.ts to reflect 1-based indexing.
    • Ensure tests verify correct parsing and line number output for various file types and scenarios.

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

@changeset-bot
Copy link

changeset-bot bot commented Apr 2, 2025

⚠️ No Changeset found

Latest commit: eb4e2b6

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 size:S This PR changes 10-29 lines, ignoring generated files. label Apr 2, 2025
@mrubens
Copy link
Collaborator Author

mrubens commented Apr 2, 2025

@KJ7LNW how does this look? Thanks!

@dosubot dosubot bot added the bug Something isn't working label Apr 2, 2025
@KJ7LNW
Copy link
Contributor

KJ7LNW commented Apr 2, 2025

lgtm.

can you also clamp startLine to go no lower than zero (at this point in the code it is always zero indexed):

src/integrations/misc/read-lines.ts-25-export function readLines(filepath: string, endLine?: number, startLine?: number): Promise<string> {

<<clamp startLine force >= 0 -- at this point in the code it is always zero indexed>>

src/integrations/misc/read-lines.ts-26- return new Promise((resolve, reject) => {
src/integrations/misc/read-lines.ts-27-         // Validate input parameters
src/integrations/misc/read-lines.ts-28-         // Check startLine validity if provided
src/integrations/misc/read-lines.ts-29-         if (startLine !== undefined && (startLine < 0 || startLine % 1 !== 0)) {
src/integrations/misc/read-lines.ts-30-                 return reject(
src/integrations/misc/read-lines.ts:31:                         new RangeError(`Invalid startLine: ${startLine}. Line numbers must be non-negative integers.`),
src/integrations/misc/read-lines.ts-32-                 )
src/integrations/misc/read-lines.ts-33-         }
src/integrations/misc/read-lines.ts-34-

it will prevent this error if the model provides 0 as a number:

<read_file>
<path>.build/linux/rpm/x86_64/rpmbuild/SPECS/code-oss.spec</path>
<start_line>0</start_line>
<end_line>50</end_line>
</read_file>

<error>
Error reading file: {"name":"RangeError","message":"Invalid startLine: -1. Line numbers must be non-negative integers.","stack":"RangeError: Invalid startLine: -1. Line numbers must be non-negative integers.\n    at /home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:1532:2592\n    at new Promise (<anonymous>)\n    at Pwe (/home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:1532:2533)\n    at MVr (/home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:2016:1666)\n    at t.presentAssistantMessage (/home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:3241:793)"}

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

mrubens commented Apr 2, 2025

lgtm.

can you also clamp startLine to go no lower than zero (at this point in the code it is always zero indexed):

src/integrations/misc/read-lines.ts-25-export function readLines(filepath: string, endLine?: number, startLine?: number): Promise<string> {

<<clamp startLine force >= 0 -- at this point in the code it is always zero indexed>>

src/integrations/misc/read-lines.ts-26- return new Promise((resolve, reject) => {
src/integrations/misc/read-lines.ts-27-         // Validate input parameters
src/integrations/misc/read-lines.ts-28-         // Check startLine validity if provided
src/integrations/misc/read-lines.ts-29-         if (startLine !== undefined && (startLine < 0 || startLine % 1 !== 0)) {
src/integrations/misc/read-lines.ts-30-                 return reject(
src/integrations/misc/read-lines.ts:31:                         new RangeError(`Invalid startLine: ${startLine}. Line numbers must be non-negative integers.`),
src/integrations/misc/read-lines.ts-32-                 )
src/integrations/misc/read-lines.ts-33-         }
src/integrations/misc/read-lines.ts-34-

it will prevent this error if the model provides 0 as a number:

<read_file>
<path>.build/linux/rpm/x86_64/rpmbuild/SPECS/code-oss.spec</path>
<start_line>0</start_line>
<end_line>50</end_line>
</read_file>

<error>
Error reading file: {"name":"RangeError","message":"Invalid startLine: -1. Line numbers must be non-negative integers.","stack":"RangeError: Invalid startLine: -1. Line numbers must be non-negative integers.\n    at /home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:1532:2592\n    at new Promise (<anonymous>)\n    at Pwe (/home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:1532:2533)\n    at MVr (/home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:2016:1666)\n    at t.presentAssistantMessage (/home/ewheeler/.vscode-insiders/extensions/rooveterinaryinc.roo-cline-3.11.3/dist/extension.js:3241:793)"}

Yeah let me do that in another PR

@mrubens mrubens merged commit 24a4669 into main Apr 2, 2025
21 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Apr 2, 2025
@mrubens mrubens deleted the fix_tree_sitter_off_by_one branch April 2, 2025 21:29
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:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants