-
Notifications
You must be signed in to change notification settings - Fork 50
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 mimetype_detection_hook. #259
Add mimetype_detection_hook. #259
Conversation
Yes I'll try it right now |
Not sure if this is asking too much, but is there a way to set precedence? For example, making it so that this function will only be used after tiled has exhausted its default mimetypes and the mimetypes_by_file_ext option? |
Ideally that order could go in any direction. |
This way I don't have to return |
Definitely not asking too much. :-) I did give precedence some thought, but I am not 100% I got it right. My thinking was that the function go first so it can get in before the file extension detector might misidentify something. I had in mind that it would look for a certain naming pattern, perhaps using regex in your case, and then just do nothing if it sees nothing it recognizes, implicitly returning None. The example in my unit test in this PR works that way: isn’t filed that don’t match what it is looking for are effectively ignored, and no extra code is required to handle that. But if for any reason you need to run the file extension detection first, you can copy that part of the tiled code directly inside your custom function, at the top. |
Perhaps instead of copying the code would it be possible to wrap the other checker in a function that I can import? |
Though this is picky on my part, and I can do it any of the other ways. |
How would I access the variable |
You'd have to duplicate it. Alternatively, we could provide it as an argument to the function, i.e. def detect_mimetype(path, mimetypes_by_file_ext):
... That occurred to me but seemed a little...overcooked. I think it depends on how frequently we need to do the mimetype check first. What does you function look like without the mimetype check? |
Without a mimetype check I would probably use a regex. But I'm not sure how guaranteed it is that all the files will have the numbers as their extension. How does duplicating it look like? Do you mean parsing the yaml within the detect_mimetype function? |
Also I am getting this error:
It seems that the mimetype_detection_hook is a string and not the actual function. |
Oops, yes I missed something important there. Please stand by. |
Fix for I meant hard-coding the dictionary of custom mimetypes in the definition of the function, duplicating whatever is in the config. Not ideal, but possibly better than the alternative of overly-magical complexity in the precedence rules. Concrete use cases will be very helpful in landing on a good design. Does your directory mix standard files like TIFF or CSV with these unusually-named ones? |
Yes, I have a csv file mixed in with them. |
It would not be too hard to parse the config file and then duplicate the mimetypes in that way. |
Thanks, it seems to be working from the python client, however for some reason the browser is now only returning 404s for the browse UI. Not sure if it's just on my end though. |
The distributions on PyPI include the pre-built React app. When you install from GitHub you need to build the React app yourself: see |
Everything seems to work! |
I thought of a new possibility that seems a better balance of flexibility and simplicity. My goals are:
What if the file extension detection runs first and then calls your hook with two parameters: the path and the mimetype. If it doesn’t match, the mimetype with be None. Therefore, if you only care about files that do not match based on the extension you can do
But, in other situations, it is still possible to override the mimetype detected based on ext if it was wrong. |
That looks perfect. Trying it out now. |
Oh wait you haven't implemented it yet lol. |
Haha, yeah. Implemented and pushed now. If it works for you, I'll update the documentation and merge. Wrote it out while it's fresh in my brain...now on to the weekend! 🌴 |
Works for me! Not sure why the tests are failing though. |
Looks like an unrelated issue started failing the unit tests. I've been meaning to adjust that anyway; fix pushed. |
89bfca0
to
c980bed
Compare
@J-avery32 The documentation you used is sort of a "case study" that covers a specific situation, and it gets into some details that might not be immediately needed by all users. I tried writing more entry-level documentation here: https://github.com/bluesky/tiled/pull/259/files#diff-2a446760a1636bbd9aae3291ce4a430b145cda1ec4fe01f33e1a5e829a8f0b2a Do you think it is understandable and useful? |
GitHub will display a more readable preview of that new docs page at https://github.com/bluesky/tiled/blob/fa42c52bc5b27c9a62fc521615999f95b3399e03/docs/source/how-to/read-custom-formats.md |
Yes it is readable to me. |
@J-avery32 Would you check out this branch and give it a try?
You need to place a Python script next to your
config.yml
file. It can be named anything:That function has access to the file path and it can do anything it wants to decide what the mimetype is, including opening the file.
To tell
DirectoryAdapter
to use it, add this to theconfig.yml
:If that function returns
None
, then theDirectoryAdapter
will fall back to its usual method of looking at file extensions.By default, Tiled will strip everything after the first
.
from the name. That is,thing.csv
will appear as justthing
. We do this to avoid "leaking" to the user details about how data happens to be stored, so that it can change over time without breaking our contract with the user.In your case, dropping everything after the
.
may not be what you want. You can add:Or write your own function in
custom.py
for transforming the filename into whatever you want to name this node in Tiled:If this works for you we'll add documentation and merge.
Closes #255
Closes #175