search

oupala / apaxy

a simple, customisable theme for your apache directory listing
https://oupala.github.io/apaxy/
GNU General Public License v3.0
1.86k stars 256 forks source link

Allow icon also for PDF #112

Closed pcav closed 6 years ago

ghost commented 6 years ago

Oh, I didn't know apache was case sensitive. In this case, should we do that for all the file extensions? Or at least some additional common ones like .ISO and .PNG?

pcav commented 6 years ago

Agreed, it would be good to replicate it for all. I didn't do it because I did not know whather this was acceptable for you, or if there is a better solution. Possibly also .Pdf etc could be added. Thanks.

oupala commented 6 years ago

I'm not a big fan of duplicating extension. It is against the DRY principle (Don't Repeat Yourself).

Lowercase extension is not enough?

There is some tools to batch-rename files.

What do you think?

pcav commented 6 years ago

It makes sense to me. On the other hand, it might be disturbing to the user, especially if there are frequent new uploads to the folder. Decision is yours :)

oupala commented 6 years ago

Decision is not mine, I'm not omnipotent.

We will close this pull request, for sure as it only concernes pdf extension.

But it is a good place to discuss about duplicating all .ext to .EXT or none.

Go and give your opinion!

pcav commented 6 years ago

Thanks. As said, I have no strong opinion (otherwise I'd have defended it as usual ;) ).

oupala commented 6 years ago

Thanks for your (not strong) opinion.

I would also like to have the opinion of other users.

ghost commented 6 years ago

Yeah, I’d say it’s too repetitive. The .htaccess file would get very messy very quickly, making it difficult to maintain.

DanielDK100 commented 6 years ago

Definitely too repetitive. I tend to use a server command for that kind of grunt work - such as rename 's/\.PDF$/\.pdf/' *.PDF to rename all extensions with .PDF to .pdf in a specific directory.

ghost commented 6 years ago

Ok. Then should we close this @oupala ? Maybe we could add something to the README about this?

oupala commented 6 years ago

I was looking at apache documentation, and I think that we could maybe try to use AddIconByType instead of AddIcon.

AddIconByType might be the solution as file.pdf and FILE.PDF should have the same mime type.

Hence this could solve the issue, choosing the icon depending on the mime type instead of the file extension.

pcav commented 6 years ago

It seems the way to go. Thanks.

oupala commented 6 years ago

Can you test it and tell us if it works?

If it does, we might change all the AddIcon to AddIconByType.

pcav commented 6 years ago

Will do ASAP

pcav commented 6 years ago

I confirm, AddIconByType (PDF,theme/icons/pdf.png) * works.

oupala commented 6 years ago

Ok, I will close this pull request and start a new one trying to replace all the AddIcon directive to AddIconByType.

Thanks for helping.