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.

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

Make chardet a optional dependency #313

Open
kasium opened this issue Jan 24, 2023 · 1 comment
Open

Make chardet a optional dependency #313

kasium opened this issue Jan 24, 2023 · 1 comment

Comments

@kasium
Copy link
Contributor

kasium commented Jan 24, 2023

I have a nice suggestion to remove the install requirements of this project: Remove the dependency on chardet.
chardet is only used in one place and there we could check if it's installed. If not, fall back to chardet.

What do you think? I'm happy to create a PR

@Bachmann1234
Copy link
Owner

The problem is that without chardet we basically have to assume the files are ascii or utf-8 encoded.

Which... ill admit... is probably fine for the overwhelming majority of users. However, I have encountered latin-1 and other encodings.

I think to really make the dependency optional I would want the following

  1. Solid error message saying something along the lines "Hey, this source file does not seem to be ascii or utf-8. You can install chardet and we can guess it"
  2. a command line option/config to specify a list of source encodings to try (defaulting to just utf-8). Because quite frankly, this would be a better option than chardet for anyone who knows they have non utf-8 source. I think it would have to be a list since its possible to have multiple encodings (technically diff cover does if you include the test fixtures :-P)

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

No branches or pull requests

2 participants