Skip to content

Loosen type restrictions in StaticFileHandler helper methods from File to IO#16238

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
andrykonchin:ak/loosen-strict-requirement-on-file-type-in-static-file-handler
Oct 23, 2025
Merged

Loosen type restrictions in StaticFileHandler helper methods from File to IO#16238
straight-shoota merged 2 commits intocrystal-lang:masterfrom
andrykonchin:ak/loosen-strict-requirement-on-file-type-in-static-file-handler

Conversation

@andrykonchin
Copy link
Contributor

@andrykonchin andrykonchin commented Oct 20, 2025

Changes

  • changed file : File parameter to io : IO in private methods StaticFileHandler#serve_file_range and StaticFileHandler#serve_file_full
  • added a comment for #serve_file_range to document that io is supposed to be seekable

Closes #15817


private def serve_file_range(context : Server::Context, file : File, range_header : String, file_info)
# *io* should be seekable, that's implement #seek method
private def serve_file_range(context : Server::Context, io : IO, range_header : String, file_info)
Copy link
Member

@straight-shoota straight-shoota Oct 20, 2025

Choose a reason for hiding this comment

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

suggestion: Maybe it would be better to keep the parameter name file? It aligns well with the method name. Conceptually, we're serving a file. It might just have a representation that's different from the File type.
Ditto below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ysbaddaden ysbaddaden added this to the 1.19.0 milestone Oct 21, 2025
@straight-shoota straight-shoota changed the title Loosen strict requirement on File type in serve_file_range and serve_file_full of StaticFileHandler to IO Loosen type restrictions in StaticFileHandler helper methods from File to IO Oct 23, 2025
@straight-shoota straight-shoota merged commit d566150 into crystal-lang:master Oct 23, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loosen strict requirement on File type in serve_file_range and serve_file_full of StaticFileHandler to IO

3 participants