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

Adds CSV MIME type that compatable with Windows #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion beangulp/importers/csv.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def prepare_for_identifier(regexps: Union[str, List[str]],
if isinstance(regexps, str):
regexps = [regexps]
matchers = matchers or []
matchers.append(('mime', 'text/csv'))
matchers.append(('mime', '(text/csv)|(application/vnd\\.ms-excel)'))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this change is that the importers based on beangulp.importers.csv will start matching all files with the Excel mime type, including xls ans xlsx files. This would be a backward compatibility break, and we should avoid this, especially for an importer base class that has been around for very very long.

What can be done is to add the mime matcher only if one has not been passed by the user already. This would allow you to add the matching of the application/vnd\\.ms-excel to your importer only. Something like this:

Suggested change
matchers.append(('mime', '(text/csv)|(application/vnd\\.ms-excel)'))
if not any(m[0] == 'mime' for m in matchers):
matchers.append(('mime', 'text/csv'))

if I remember correctly how this code is supposed to work.

if regexps:
for regexp in regexps:
matchers.append(('content', regexp))
Expand Down