Skip to content

Conversation

@thiemowmde
Copy link

This was disabled in 2023 for unknown reasons. Can we try to enable it again and see what happens?

This was disabled in 2023 for unknown reasons. Can we try to enable it again and see what happens?
Copy link
Collaborator

@tacsipacsi tacsipacsi left a comment

Choose a reason for hiding this comment

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

It was actually disabled probably much earlier, before I started contributing here: c879212 only brought the already-existing code changes to GitHub. I’d appreciate if @Steinsplitter could comment if they have any idea why it was disabled.

else if(substr(strtolower($picture['title']),-4) == ".png") { $image->filetype = "png"; }
else if(substr(strtolower($picture['title']),-4) == ".gif") { $image->filetype = "gif"; }
// if(substr(strtolower($picture['title']),-5) == ".tiff" OR substr(strtolower($picture['title']),-4) == ".tif") { $image->filetype = "tif"; }
if(preg_match('/\.tiff?$/i',$picture['title'])) { $image->filetype = "tif"; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I’d keep the code consistent: either preg_match() for all file types or substr(strtolower()) for all file types.

Copy link
Author

Choose a reason for hiding this comment

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

Please feel free to upload another patch that does this or edit this patch accordingly. I found it important to keep this patch as small as possible so it's very obvious what it's supposed to do, and very easy to revert if needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with keeping the change as small as possible to make it easier to revert. However, there are two ways to ensure consistency, and only one of them makes the diff larger – the other one actually makes it even smaller and even more focused.

Copy link
Author

Choose a reason for hiding this comment

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

As said, please feel free to edit this patch.

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