Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix Unexpected Newline in Output when Copying Empty Files #31

Closed
supitsdu opened this issue Jun 28, 2024 · 0 comments · Fixed by #35
Closed

Fix Unexpected Newline in Output when Copying Empty Files #31

supitsdu opened this issue Jun 28, 2024 · 0 comments · Fixed by #35
Assignees
@supitsdu
Labels
bug Something isn't working as expected refactoring Issues or PRs to improving code structure, without changing its external behavior testing Issues or PRs specifically related to Go testing frameworks or practices.

Comments

@supitsdu
Copy link
Owner

During a code review of Pull Request #29, it was discovered that when ParseContent is used to copy content from an empty file, the output contains an unexpected newline character (\n) instead of an empty string. This behavior was observed in the following scenario:

  • The readers slice passed to ParseContent contains a FileContentReader where the FilePath points to an empty file.
  • The function returns a string containing only a newline character ("\n") instead of an empty string ("").

Expected Behavior:

When copying content from an empty file, ParseContent should return an empty string to accurately reflect the lack of content in the file.

Proposed Fix:

Add a check within the loop in ParseContent to ensure that the content read from each reader is not empty before appending a newline character and writing it to the strings.Builder.

Additional Notes:

  • Review the unit tests for ParseContent to ensure they cover the case of empty file input.
  • Consider adding a test specifically for this scenario to prevent regressions in the future.

Originally posted by @ccoVeille in #29 (comment)
@supitsdu supitsdu added bug Something isn't working as expected testing Issues or PRs specifically related to Go testing frameworks or practices. refactoring Issues or PRs to improving code structure, without changing its external behavior labels Jun 28, 2024
@supitsdu supitsdu self-assigned this Jun 30, 2024
supitsdu added a commit that referenced this issue Jun 30, 2024
@supitsdu
fix: Prevent unexpected newline in output for empty files
9a59b6c 
Modified ReadContentConcurrently and AggregateContent functions in reader package to handle empty content scenarios gracefully. ReadContentConcurrently now checks for empty content before appending to results, ensuring only valid content is processed. AggregateContent skips appending newline separators for empty content, addressing the issue where empty files would erroneously contribute newlines to the final output.

- [x] Review the unit tests for ParseContent to ensure they cover the case of empty file input.
- [ ] Consider adding a test specifically for this scenario to prevent regressions in the future.

Closes #31
@supitsdu supitsdu mentioned this issue Jun 30, 2024
Merged
2 tasks
supitsdu added a commit that referenced this issue Jun 30, 2024
@supitsdu
test(reader): Prevent regression with empty files
a645436 
Updated tests in reader_test.go to include checks for handling empty files in ParseContent function of reader package. Added tests to verify that ParseContent correctly returns an empty string when provided with an empty file path and a newline character string. This ensures consistent behavior and addresses potential issues highlighted in #31.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@supitsdu supitsdu

Labels
bug Something isn't working as expected refactoring Issues or PRs to improving code structure, without changing its external behavior testing Issues or PRs specifically related to Go testing frameworks or practices.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Prevent unexpected newline in output for empty files supitsdu/clipper
1 participant
@supitsdu