fix(next/diff): String::split API breaking change
Ensure string ownership with explicit to_string() conversion
Category Correctness Code Snippet lines.push(Line::new(line_number, line.to_string())) Recommendation The current change is correct. Keep the explicit to_string() conversion to ensure proper string ownership. Reasoning The split() operation likely returns string slices/views. When constructing Line objects that need to own their content, explicit conversion to owned strings prevents potential lifetime/ownership issues.
Duplicated code across multiple files
Category Maintainability Code Snippet Files: part1/line.mbt, part2/line.mbt, part3/line.mbt contain identical code Recommendation Consider refactoring the common line handling logic into a shared module that can be used by all three parts Reasoning Having identical code in multiple places increases maintenance burden and risk of inconsistencies when changes are needed. A shared implementation would be easier to maintain and ensure consistency.
Missing documentation for Line constructor and lines function
Category Maintainability Code Snippet fn lines(str : String) -> Array[Line] { Recommendation Add documentation comments explaining the purpose and behavior of the Line::new constructor and lines function Reasoning Documentation helps other developers understand the purpose and usage of the code, especially for public functions that likely form part of the module's API
I would rather change it the other way round: make Line.text @string.View