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

Add support for braille format files #10970

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

edwilde
Copy link
Contributor

@edwilde edwilde commented Oct 11, 2023

This adds support for the Braille ASCII file format.

This is increasingly requested by clients in the public sector.

Possible caveat: It seems, there is disagreement on what mime type brf files should be: datatypes.net says it should be application/octet-stream (ref), but apache says it should be text/plain (ref). In custom configurations we typically add both.

In this case I believe the apache decision pre-dates datetypes.net (I only see May 2022 in the wayback machine) and makes more sense given the type of content within the file.

Issue

This adds support for the Braille ASCII file format. This is increasingly requested by clients in the public sector.
@GuySartorelli
Copy link
Member

Regarding the chosen mimetype

I've downloaded a .brf file from https://covid19.govt.nz/languages-and-resources/alternate-formats/braille/ and run it through some checkers to see what mime type they think it should be, and everything comes back text/plain, so I agree that seems to be the appropriate mime type. No idea where datatypes.net are getting theirs from.

Is this what you wanted?

For context, when you say "adds support", this is adding support for the HTTPRequest::send_file() method (which is about the only thing which uses HTTP::get_mime_type() which is what consumes this config) and not much else.

Is this the place (and the only place) you are intending to add support?

@edwilde
Copy link
Contributor Author

edwilde commented Oct 11, 2023

Is this the place (and the only place) you are intending to add support?

Ahha...thanks for pointing that out, I will create another couple for File and MimeValidator, which should complete the trifecta 👍

@GuySartorelli
Copy link
Member

Awesome. Can you please also create a new issue (probably against silverstripe/silverstripe-assets as the main "file" repo) to track all the related PRs? I'd like to review them all together.

@edwilde
Copy link
Contributor Author

edwilde commented Oct 11, 2023

Added silverstripe/silverstripe-assets#573

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Thanks for this.

@GuySartorelli GuySartorelli merged commit f9986c5 into silverstripe:5 Oct 25, 2023
15 checks passed
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

Successfully merging this pull request may close these issues.

2 participants