Skip to content

Commit

Permalink
fix: Prevent unexpected newline in output for empty files (#35)
Browse files Browse the repository at this point in the history
* fix: Prevent unexpected newline in output for empty files

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.
- [x] Consider adding a test specifically for this scenario to prevent regressions in the future.

Closes #31

* test(reader): Prevent regression with empty files

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.
  • Loading branch information
supitsdu authored Jun 30, 2024
1 parent 7ddb223 commit a1e6216
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
13 changes: 9 additions & 4 deletions cli/reader/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@ func ReadContentConcurrently(readers []ContentReader) ([]string, error) {
errChan <- err // Send error to channel
return
}
mu.Lock()
results[i] = content // Safely write to results slice
mu.Unlock()

if content != "" { // Check for empty content before writing to results
mu.Lock()
results[i] = content // Safely write to results slice
mu.Unlock()
}
}(i, reader)
}

Expand All @@ -79,7 +82,9 @@ func ReadContentConcurrently(readers []ContentReader) ([]string, error) {
func AggregateContent(results []string) string {
var sb strings.Builder
for _, content := range results {
sb.WriteString(content + "\n")
if content != "" { // Ensure non-empty content is aggregated
sb.WriteString(content + "\n")
}
}
return sb.String()
}
Expand Down
12 changes: 8 additions & 4 deletions tests/reader/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ func TestParseContent(t *testing.T) {
}

// Expected result when parsing an empty file
expected := "\n"
emptyString := ""
newlineString := "\n"

// Create a FileContentReader for the empty file
testReader := reader.FileContentReader{FilePath: emptyFile.Name()}
Expand All @@ -115,9 +116,12 @@ func TestParseContent(t *testing.T) {
t.Fatalf("Expected no error, got: %v", err)
}

// Verify the actual output matches the expected output
if actual != expected {
t.Errorf("Expected %s but got %s", expected, actual)
if actual != emptyString {
t.Error("When given an empty file returned something else.")
}

if actual == newlineString {
t.Error("When given empty file returned one with newline '\\n' character. See: https://github.com/supitsdu/clipper/issues/31#issue-2379691170")
}
})

Expand Down

0 comments on commit a1e6216

Please sign in to comment.