IO: Store last file select directory info in file handlers #116999

Guillermo Venegas · 2024-01-10T20:09:56+01:00

Guillermo Venegas commented

2024-01-10 20:09:56 +01:00

Currently the filesel window depends on operator filepath
properties to allow reuse last operator import/export paths
to open automatically the last folder used by the user, and this
depends that the properties does not have a PROP_SKIP_SAVE flag.

However this has 2 drawbacks:

Since operators can be invoked with drag path data,
operator presets will override this filepath information.
If this filepath properties are defined with update function callbacks,
operator presets would override filepath properties and trigger this callbacks,
but when the user click import/export the operator filepath properties are updated again
with the selected fielpath data from the filesel window, that could lead inconsistent behavior.

Instead now the operator will keep track of the last directory used on it owns file handler.

Currently the filesel window depends on operator filepath properties to allow reuse last operator import/export paths to open automatically the last folder used by the user, and this depends that the properties does not have a `PROP_SKIP_SAVE` flag. However this has 2 drawbacks: 1. Since operators can be invoked with drag path data, operator presets will override this filepath information. 2. If this filepath properties are defined with update function callbacks, operator presets would override filepath properties and trigger this callbacks, but when the user click `import/export` the operator filepath properties are updated again with the selected fielpath data from the filesel window, that could lead inconsistent behavior. Instead now the operator will keep track of the last directory used on it owns file handler.

Guillermo Venegas added 1 commit 2024-01-10 20:10:01 +01:00

3d7df64565 init

Guillermo Venegas added 1 commit 2024-01-10 20:21:15 +01:00

a902809189 format

Guillermo Venegas referenced this pull request

2024-01-10 20:32:24 +01:00

IO: Add file handlers to c++ import operators #116873

Guillermo Venegas added 1 commit 2024-01-10 22:13:15 +01:00

571713c855 explicit requested path

Brecht Van Lommel commented

2024-01-12 14:37:29 +01:00

I don't really understand this problem.

Since operators can be invoked with drag path data, operator presets will override this filepath information.

To me it seems like reasonable behavior for drag & drop to affect this.

If this filepath properties are defined with update function callbacks, operator presets would override filepath properties and trigger this callbacks, but when the user click import/export the operator filepath properties are updated again with the selected fielpath data from the filesel window, that could lead inconsistent behavior.

Is this an actual problem you encountered in an add-on?

I don't really understand what this inconsistent behavior is that would be triggered by calling update twice.

I don't really understand this problem. > Since operators can be invoked with drag path data, operator presets will override this filepath information. To me it seems like reasonable behavior for drag & drop to affect this. > If this filepath properties are defined with update function callbacks, operator presets would override filepath properties and trigger this callbacks, but when the user click import/export the operator filepath properties are updated again with the selected fielpath data from the filesel window, that could lead inconsistent behavior. Is this an actual problem you encountered in an add-on? I don't really understand what this inconsistent behavior is that would be triggered by calling update twice.

Guillermo Venegas commented

2024-01-12 17:15:42 +01:00

This is the 1st problem:

Preset would override drag-n-drop data

Is this an actual problem you encountered in an add-on?

Not exactly, but let's say someone wants to create a addon that imports files, but the operator requires to read metadata from the file and adds more menu settings as needed. If presets are selected will override operator filepath props, and and probably read metadata from another file.
This is resolved by adding skip_save to props and the presets will not override these neither or trigger callbacks, but with skip_save the fileselect window would not remeber last location where this operator was used.
Although currently internal operators that read files only use the ot->check to validate extension, so I don't know if it could be a very strong point
But maybe the preset at all would not work too well on this scenarios

This is the 1st problem: <video src="/attachments/72be36e8-9525-4617-a751-ca115bc1105b" title="2024-01-09 16-49-58.mp4" controls></video> Preset would override drag-n-drop data >Is this an actual problem you encountered in an add-on? Not exactly, but let's say someone wants to create a addon that imports files, but the operator requires to read metadata from the file and adds more menu settings as needed. If presets are selected will override operator filepath props, and and probably read metadata from another file. This is resolved by adding `skip_save` to props and the presets will not override these neither or trigger callbacks, but with `skip_save` the fileselect window would not remeber last location where this operator was used. Although currently internal operators that read files only use the `ot->check` to validate extension, so I don't know if it could be a very strong point But maybe the preset at all would not work too well on this scenarios

Brecht Van Lommel requested review from Jesse Yurkovich 2024-01-15 20:06:08 +01:00

Brecht Van Lommel requested review from Brecht Van Lommel 2024-01-15 20:07:09 +01:00

Brecht Van Lommel commented

2024-01-15 20:11:04 +01:00

Thansk for the explanation, I can see the motivation. It still seems like a bit of a workaround for the preset problem. Maybe we need a skip_preset to exclude the filepath from the preset but still save it?

@deadpin, do you have an opinion on this?

Thansk for the explanation, I can see the motivation. It still seems like a bit of a workaround for the preset problem. Maybe we need a `skip_preset` to exclude the filepath from the preset but still save it? @deadpin, do you have an opinion on this?

Guillermo Venegas commented

2024-01-15 20:26:49 +01:00

#116724 Resolves in some way the problem that operator presets override filepath props
Because on confirm execution the file path properties are set

Not exactly, but let's say someone wants to create a addon that imports files, but the operator requires to read metadata from the file and adds more menu settings as needed. If presets are selected will override operator filepath props, and and probably read metadata from another file.

This would be something that might not be so supported with #116724.
Specially since the configuration of one file is reused for all other files (in case a operator does not support multiple files ot->exec is called for each individually)

#116724 Resolves in some way the problem that operator presets override filepath props Because on confirm execution the file path properties are set >Not exactly, but let's say someone wants to create a addon that imports files, but the operator requires to read metadata from the file and adds more menu settings as needed. If presets are selected will override operator filepath props, and and probably read metadata from another file. This would be something that might not be so supported with #116724. Specially since the configuration of one file is reused for all other files (in case a operator does not support multiple files `ot->exec` is called for each individually)

Jesse Yurkovich commented

2024-01-15 20:45:53 +01:00

Right, the Preset issue is what's holding up #116873 mostly. If the file path information was never stored in the preset file to begin with, we wouldn't have run into the problem.

I like the concept of using a skip_preset flag too since that removes the need to store the paths again here. I don't think anyone would be relying on the paths inside the preset since the File browser always uses its path information anyways but hard to know for sure. A downside of skip_preset is that existing preset files would trigger wrong behavior with drag'n'drop though again I'm not sure how many folks save preset files like that across versions.

Right, the Preset issue is what's holding up #116873 mostly. If the file path information was never stored in the preset file to begin with, we wouldn't have run into the problem. I like the concept of using a `skip_preset` flag too since that removes the need to store the paths again here. I don't think anyone would be relying on the paths inside the preset since the File browser always uses its path information anyways but hard to know for sure. A downside of `skip_preset` is that _existing_ preset files would trigger wrong behavior with drag'n'drop though again I'm not sure how many folks save preset files like that across versions.

Jesse Yurkovich commented

2024-01-15 21:27:26 +01:00

Actually the last part about existing preset files applies in all cases so that doesn't matter too much.

Both the skip_preset option and the other WM_FILESEL_SKIP_SAVE_PROPS option added in the other PR will result in not writing file path information into the preset file. So from that angle, skip_preset (WM_FILESEL_SKIP_SAVE_PRESET or similar) seems better since it doesn't require us to store the path information in the FileHandler or modify the file browser (I think).

Actually the last part about existing preset files applies in all cases so that doesn't matter too much. Both the `skip_preset` option and the other `WM_FILESEL_SKIP_SAVE_PROPS` option added in the other PR will result in not writing file path information into the preset file. So from that angle, `skip_preset` (`WM_FILESEL_SKIP_SAVE_PRESET` or similar) seems better since it doesn't require us to store the path information in the FileHandler or modify the file browser (I think).

Guillermo Venegas commented

2024-01-15 23:18:26 +01:00

Actually, adding PROP_HIDDEN also prevents writing to presets and allows to reuse property values between calls, and since they're also hidden to the user...

Closing since is not too necessary this, however skip_preset can be useful to have

Actually, adding `PROP_HIDDEN` also prevents writing to presets and allows to reuse property values between calls, and since they're also hidden to the user... ![image](/attachments/e43fc21b-9955-4324-9caf-6ba04825a446) Closing since is not too necessary this, however `skip_preset` can be useful to have

image.png

79 KiB

Guillermo Venegas closed this pull request

2024-01-15 23:18:28 +01:00

Guillermo Venegas deleted branch fh-last-dirpath

2024-01-15 23:21:53 +01:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

IO: Store last file select directory info in file handlers #116999

Pull request closed