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

Automatically generate file props from path #360

Closed
wants to merge 1 commit into from

Conversation

aschempp
Copy link
Member

@aschempp aschempp commented Sep 9, 2024

would be quite cumbersome if everyone has to do that themselve?

@aschempp aschempp requested a review from Toflar September 9, 2024 19:08
}

if (null === $mimeType) {
$mimeType = mime_content_type($path);
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be use the mime type guesser from Symfony here instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

any hint what that looks like?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could instantiate it yourself via (new MimeTypes())->guessMimeType($path). However, the symfony/framework-bundle also registers it as the mime_types service, which allows the system to add additional guessers via the MimeTypeGuesserInterface via the AddMimeTypeGuesserPass compiler pass. This would have to be done via a helper service outside this static function.

@Toflar
Copy link
Member

Toflar commented Sep 9, 2024

I would like to have a separate helper method. Less magic, more expressive.

@aschempp
Copy link
Member Author

aschempp commented Sep 9, 2024

but that method already is a helper method to create a FileItem?

@Toflar
Copy link
Member

Toflar commented Sep 10, 2024

Yes but right now, null is not allowed - checks have to be applied outside of the method. You are now moving them into the factory method and there are not even nullable checks.

@Toflar
Copy link
Member

Toflar commented Sep 11, 2024

I think we should introduce a FileItemFactory service instead which provides multiple helpers for creating a FileItem and can thus also access the Symfony MimeTypeGuesser service. Also, I would like to look into providing a helper for creating a FileItem based on a FilesystemItem from Contao's VFS.
Tagging this as enhancement for 2.1.

@Toflar Toflar added this to the 2.1 milestone Sep 11, 2024
@Toflar Toflar mentioned this pull request Sep 17, 2024
@Toflar
Copy link
Member

Toflar commented Sep 17, 2024

Closing in favor of #362.

@Toflar Toflar closed this Sep 17, 2024
@aschempp aschempp deleted the feature/detect-file branch September 26, 2024 06:57
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.

3 participants