-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add optional line numbering to read_text_file tool #61
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
base: main
Are you sure you want to change the base?
feat: add optional line numbering to read_text_file tool #61
Conversation
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.
Hey @abderraouf-belalia , thanks for the contribution. this is a good enhancement.
I requested some small changes, I will review it after those are addressed.
CHANGELOG.md
Outdated
| ## [Unreleased] | ||
|
|
||
| ### 🚀 Features | ||
|
|
||
| * Add optional line numbering to read_text_file tool ([#60](https://github.com/rust-mcp-stack/rust-mcp-filesystem/issues/60)) | ||
| - Added `with_line_numbers` optional parameter to `read_text_file` tool | ||
| - When enabled, prefixes each line with right-aligned line numbers and pipe separator | ||
| - Useful for AI agents that need to target specific lines for code patches | ||
| - Maintains backward compatibility with existing usage | ||
|
|
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.
Updating the CHANGELOG.md is not necesarry :) , it will be auto-generated at release time.
docs/capabilities.md
Outdated
| <code><b>read_text_file</b></code> | ||
| </td> | ||
| <td>Read the complete contents of a text file from the file system as text. Handles various text encodings and provides detailed error messages if the file cannot be read. Use this tool when you need to examine the contents of a single file. Only works within allowed directories.</td> | ||
| <td>Read the complete contents of a text file from the file system as text. Handles various text encodings and provides detailed error messages if the file cannot be read. Use this tool when you need to examine the contents of a single file. Optionally include line numbers for precise code targeting. Only works within allowed directories.</td> |
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.
This file will be automatically generated using mcp-discovery
src/tools/read_text_file.rs
Outdated
| /// When enabled, each line is prefixed with its line number (1-based). | ||
| /// Useful for AI agents that need to target specific lines for code patches. |
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.
This makes it easier for AI systems to understand the format , more concise, precise, and clear.
| /// When enabled, each line is prefixed with its line number (1-based). | |
| /// Useful for AI agents that need to target specific lines for code patches. | |
| /// When enabled, each line is prefixed with a right-aligned, 1-based line number | |
| /// Followed by a space, a vertical bar (`|`), and another space in the format: ` 123 | <original line content>` |
| let file_path = create_temp_file( | ||
| temp_dir.join("dir1").as_path(), | ||
| "test.txt", | ||
| "line1\nline2\nline3", |
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.
Would you mind adding similar test case that uses \r\n instead of just \n? to test the windows line ending?
| #[tokio::test] | ||
| async fn test_read_text_file_with_line_numbers_empty_file() { | ||
| let (temp_dir, service, _allowed_dirs) = setup_service(vec!["dir1".to_string()]); | ||
| let file_path = create_temp_file(temp_dir.join("dir1").as_path(), "empty.txt", ""); |
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.
Can you please add two more tests for some edge cases?
one test for a file with only one \n in the content , and another one with a \r\n content
both should read:
1 |
2 |
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.
Hey!
Regarding the tests with a file with only one \n or \r\n, it wouldn't generate 2 empty lines per my understanding and some playground confirmations but a single empty due to how .lines() works as it splits the file using \n as a separator and "\n" is seen as "[EMPTY STRING]\n" rather than "[EMPTY STRING]\n[EMPTY STRING]". The only way to get,
1 |
2 |
is to write the tests with \n\n (similar logic for windows).
- Remove manual CHANGELOG.md entry (auto-generated at release) - Revert docs/capabilities.md changes (auto-generated via mcp-discovery) - Improve documentation clarity for with_line_numbers parameter format - Add test for Windows line endings (\r\n) - Add edge case tests for newline-only content Addresses review comments on PR rust-mcp-stack#61 [agent commit]
Implements rust-mcp-stack#60 - Add line numbering flag for read_text_file Changes: - Added `with_line_numbers` optional parameter to ReadTextFile struct - Updated read_text_file service method to format output with line numbers - Line numbers are right-aligned (6 digits) with pipe separator format - Uses 1-based indexing for line numbers - Maintains backward compatibility (defaults to false) - Added comprehensive unit tests for various scenarios - Updated CHANGELOG.md with feature description This feature enables AI agents to obtain file content with line numbers in a single tool invocation, improving efficiency for code modification tasks that require precise line-based targeting. [agent commit]
[agent commit]
[agent commit]
- Remove manual CHANGELOG.md entry (auto-generated at release) - Revert docs/capabilities.md changes (auto-generated via mcp-discovery) - Improve documentation clarity for with_line_numbers parameter format - Add test for Windows line endings (\r\n) - Add edge case tests for newline-only content Addresses review comments on PR rust-mcp-stack#61 [agent commit]
ffbc6e5 to
7300ee3
Compare
[agent commit]
Summary
Implements #60 - Adds an optional
with_line_numbersparameter to theread_text_filetool to support line-numbered output for AI agents.Changes
with_line_numbersoptional boolean parameter toReadTextFilestructread_text_fileservice method to format output with line numbers when enabled1 | contentfalse)Motivation
Modern AI agents (like Codex and others) rely on line numbers to inject targeted patches to code. This feature allows agents to obtain all necessary context in a single tool invocation rather than making multiple calls, significantly improving efficiency for code modification tasks.
Example Output
Without line numbers (default):
With line numbers (
with_line_numbers: true):Test Plan
Testing
Run tests with:
cargo test --test test_fs_service test_read_text_fileAll 6 new tests pass successfully.
Validation
All project checks pass:
[agent pull request]