IO: Add support for multiple drag-n-drop files #107230

Merged
Brecht Van Lommel merged 20 commits from guishe/blender:dragndrop-files into main 2023-12-12 18:46:22 +01:00
1 changed files with 2 additions and 1 deletions
Showing only changes of commit 7aede01d90 - Show all commits

View File

@ -765,7 +765,8 @@ wmDragPath *WM_drag_create_path_data(blender::Span<const char *> _paths)
const char *ext = BLI_path_extension(_paths[0]);
blender::Vector<std::string> paths;
for (auto path : _paths) {
if (STREQ(ext, BLI_path_extension(path))) {
const char *test_ext = BLI_path_extension(path);
brecht marked this conversation as resolved Outdated

Is there a reason for using string extension instead of the (already computed) file_type here (and then comparing to the result of ED_path_extension_type for all other paths in the loop below)?

If yes, should be documented in a comment.

Current approach would also 'fail' (filter out valid paths) e.g. in case of paths to a mix of JPG and PNG images. Or paths containing several USD files with different extensions.

Is there a reason for using string extension instead of the (already computed) `file_type` here (and then comparing to the result of `ED_path_extension_type` for all other paths in the loop below)? If yes, should be documented in a comment. Current approach would also 'fail' (filter out valid paths) e.g. in case of paths to a mix of JPG and PNG images. Or paths containing several USD files with different extensions.

ED_path_extension_type is only for file types that Blender natively supports, while this system is meant to work for arbitrary add-ons.

`ED_path_extension_type` is only for file types that Blender natively supports, while this system is meant to work for arbitrary add-ons.

Initially I had the idea of just the one extension for file handler, but since file handlers could have a list of files they can take, here we can copy all and then see which ones are useful for the file handlers.

path_data->file_type would only be used internally with the first file, and since these changes don't change the behavior I think it's safe to keep it and collect all the file paths, I'll leave a note that this file_type will just you describe the first path.

Initially I had the idea of just the one extension for file handler, but since file handlers could have a list of files they can take, here we can copy all and then see which ones are useful for the file handlers. `path_data->file_type` would only be used internally with the first file, and since these changes don't change the behavior I think it's safe to keep it and collect all the file paths, I'll leave a note that this `file_type` will just you describe the first path.

Ah yes, makes sense, better not do any filtering here then indeed.

Ah yes, makes sense, better not do any filtering here then indeed.
if (ext == test_ext || (ext && test_ext && STREQ(ext, test_ext))) {
paths.append(path);
guishe marked this conversation as resolved Outdated

This is just const char* isn't it? In that case auto just hides type information with no real benefit, prefer not using it in such cases.

This is just `const char*` isn't it? In that case `auto` just hides type information with no real benefit, prefer not using it in such cases.
}
}