Improve error handling

[Mahdiglm]

Adds more context to error messages

[spenserblack]

The most common point of confusion I've seen is languages that are excluded by default not being shown. For example, JSON, a data language, is excluded by default, so a repo that is only JSON files would have "no source code" unless they use the -T option. Perhaps you should mention that instead.

[spenserblack]

Why are we allowing dead code here?

[Mahdiglm]

hey @spenserblack thank you for your feedback.
About the language detection message, that's a good point about data languages. I was just trying to improve the error message but didn't know that was actually the more common issue users run into. I'd be happy to update it to mention the "-T" option instead of the hidden files thing.
And for the #[allow(dead_code)] I added that because the compiler was complaining about FileChurn::new being unused in the actual codebase. It's only used in the tests (we're using direct struct initialization in compute_file_churns). Just seemed cleaner than rewriting all the tests or changing the implementation. But if you'd prefer a different approach I'm open to it.

I'm actually still learning Rust, so I Thanks for the guidance! Should I update the PR with the language suggestion?

[o2sh]

Why did you change file_path to file_name?

[Mahdiglm]

Actually, I didn't change the behavior there - the original code was already extracting and storing just the file name. I just switched from direct struct initialization to using the constructor method.
The original code was:

let file_name = path.split('/').last().unwrap_or(path); Some(FileChurn { file_path: file_name.to_string(), nbr_of_commits: *count, number_separator, })

And I changed it to:

let file_name = path.split('/').last().unwrap_or(path); Some(FileChurn::new( file_name.to_string(), *count, number_separator, ))

The variable name file_name was already there, and it was already being assigned to the file_path field. I just switched to using the constructor to eliminate the "unused method" warning since FileChurn::new() wasn't being used in the main code (only in tests).

[o2sh]

I'm confused, this is not what I see in the diff.

[Mahdiglm]

ah sorry I misclicked start a review button

[spenserblack]

While improving the error messages to provide more info is a good idea, I think this PR does too much.
Besides what's mentioned in #1560 (comment), I'm seeing variables and imports being renamed when that's not really relevant to the error handling IMO.

The best PR does only what it says, and doesn't include possibly unrelated changes.

[spenserblack]

And for the #[allow(dead_code)] I added that because the compiler was complaining

When the compiler complains, it's best to handle the complaint instead of silence it. In this case, if the code is only used in tests, then you can move the implementation to inside the test scope.