Skip to content

feat: improve file diff for fs write#799

Merged
brandonskiser merged 3 commits intomainfrom
bskiser/file-diff
Mar 12, 2025
Merged

feat: improve file diff for fs write#799
brandonskiser merged 3 commits intomainfrom
bskiser/file-diff

Conversation

@brandonskiser
Copy link
Copy Markdown
Contributor

@brandonskiser brandonskiser commented Mar 11, 2025

Description of changes:

  • Adding a file diff visualization for fs_write that displays line numbers. This implementation needs to be improved for truecolor support - currently, we don't highlight the entire document which can result in lines being highlighted incorrectly, consequently impacting the diff.
  • Removing the clean_file_content function since it was resulting in a lot of errors with the model trying to use str_replace. Essentially, we were writing different content than the model was generating, which thus required it to constantly reread the file to see what we actually wrote.
  • Moving the logic for appending a newline to a separate function shared by all fs_write commands.
  • Fixed bug with append not including a newline at the end of the file.
Screenshot 2025-03-11 at 2 55 08 PM Screenshot 2025-03-11 at 2 56 20 PM

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 11, 2025

Codecov Report

❌ Patch coverage is 60.00000% with 112 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.45%. Comparing base (b9d7bbc) to head (fda6e4a).
⚠️ Report is 829 commits behind head on main.

Files with missing lines Patch % Lines
crates/q_cli/src/cli/chat/tools/fs_write.rs 59.78% 90 Missing and 21 partials ⚠️
crates/q_cli/src/cli/chat/tools/mod.rs 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #799      +/-   ##
==========================================
+ Coverage   13.42%   13.45%   +0.03%     
==========================================
  Files        2359     2359              
  Lines      203091   203141      +50     
  Branches   183455   183505      +50     
==========================================
+ Hits        27272    27342      +70     
+ Misses     174417   174383      -34     
- Partials     1402     1416      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@brandonskiser brandonskiser marked this pull request as ready for review March 11, 2025 23:18
@brandonskiser brandonskiser requested a review from a team March 11, 2025 23:18
@brandonskiser brandonskiser merged commit d51f2fd into main Mar 12, 2025
20 checks passed
@brandonskiser brandonskiser deleted the bskiser/file-diff branch March 12, 2025 03:14
jlhood pushed a commit to jlhood/amazon-q-developer-cli that referenced this pull request Mar 16, 2025
* feat: improve file diff for fs write

* remove clean file, minor refactoring

* refactoring, use shell_color to determine truecolor support
@hsbakshi
Copy link
Copy Markdown
Contributor

Is there a way we could remove the trailing whitespace that the model generates? It results in several checkstyle/lint errors as well as some people have editors that highlight them as errors.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants