Fix C operators can't set default display or sort type for File Browser

`WM_operator_properties_filesel()` allows C operators to set a display or sort
type for the File Browser to use. But the File Browser would always override
that because of an invalid `_is_set()` check. (The operators don't actually set
the value, they only set the property's default value.)

The only operator affected by this is "Recover Auto Save". It is supposed to
show a vertical list ordered chronologically. It used settings from the
previous File Browser usage before this patch.

Operators using the File Browser should generally use
`FILE_DEFAULTDISPLAY`/`FILE_SORT_DEFAULT` now, except if they have a reason not
to. See comments at their definition.

----

This makes it so operators that set a different display or sort type
don't change the sort or display type for the next File Browser operation.
So using "Recover Auto Save" entirely isolates display and sort type from other
operations.

Differential Revision: https://developer.blender.org/D8598

Reviewed by: Bastien Montagne

3 files changed, 60 insertions, 29 deletions

diff --git a/source/blender/editors/space_file/file_ops.c b/source/blender/editors/space_file/file_ops.c
index 7cfd749d013..93367ad3d3c 100644
--- a/source/blender/editors/space_file/file_ops.c
+++ b/source/blender/editors/space_file/file_ops.c
@@ -1351,18 +1351,17 @@ static int file_column_sort_ui_context_invoke(bContext *C,
     if (column_type != COLUMN_NONE) {
       const FileAttributeColumn *column = &sfile->layout->attribute_columns[column_type];
 
-      if (column->sort_type != FILE_SORT_NONE) {
-        if (sfile->params->sort == column->sort_type) {
-          /* Already sorting by selected column -> toggle sort invert (three state logic). */
-          sfile->params->flag ^= FILE_SORT_INVERT;
-        }
-        else {
-          sfile->params->sort = column->sort_type;
-          sfile->params->flag &= ~FILE_SORT_INVERT;
-        }
-
-        WM_event_add_notifier(C, NC_SPACE | ND_SPACE_FILE_PARAMS, NULL);
+      BLI_assert(column->sort_type != FILE_SORT_DEFAULT);
+      if (sfile->params->sort == column->sort_type) {
+        /* Already sorting by selected column -> toggle sort invert (three state logic). */
+        sfile->params->flag ^= FILE_SORT_INVERT;
       }
+      else {
+        sfile->params->sort = column->sort_type;
+        sfile->params->flag &= ~FILE_SORT_INVERT;
+      }
+
+      WM_event_add_notifier(C, NC_SPACE | ND_SPACE_FILE_PARAMS, NULL);
     }
   }
 
diff --git a/source/blender/editors/space_file/filelist.c b/source/blender/editors/space_file/filelist.c
index 465f05246ca..24b8d4deca0 100644
--- a/source/blender/editors/space_file/filelist.c
+++ b/source/blender/editors/space_file/filelist.c
@@ -583,7 +583,7 @@ static int compare_extension(void *user_data, const void *a1, const void *a2)
 
 void filelist_sort(struct FileList *filelist)
 {
-  if ((filelist->flags & FL_NEED_SORTING) && (filelist->sort != FILE_SORT_NONE)) {
+  if (filelist->flags & FL_NEED_SORTING) {
     void *sort_cb = NULL;
 
     switch (filelist->sort) {
@@ -599,7 +599,7 @@ void filelist_sort(struct FileList *filelist)
       case FILE_SORT_EXTENSION:
         sort_cb = compare_extension;
         break;
-      case FILE_SORT_NONE: /* Should never reach this point! */
+      case FILE_SORT_DEFAULT:
       default:
         BLI_assert(0);
         break;
@@ -1562,7 +1562,6 @@ void filelist_free(struct FileList *filelist)
   memset(&filelist->filter_data, 0, sizeof(filelist->filter_data));
 
   filelist->flags &= ~(FL_NEED_SORTING | FL_NEED_FILTERING);
-  filelist->sort = FILE_SORT_NONE;
 }
 
 void filelist_freelib(struct FileList *filelist)
diff --git a/source/blender/editors/space_file/filesel.c b/source/blender/editors/space_file/filesel.c
index 5d90403937a..15c6972c5f5 100644
--- a/source/blender/editors/space_file/filesel.c
+++ b/source/blender/editors/space_file/filesel.c
@@ -267,15 +267,16 @@ short ED_fileselect_set_params(SpaceFile *sfile)
       params->display = RNA_property_enum_get(op->ptr, prop);
     }
 
+    if (params->display == FILE_DEFAULTDISPLAY) {
+      params->display = U_default.file_space_data.display_type;
+    }
+
     if ((prop = RNA_struct_find_property(op->ptr, "sort_method"))) {
       params->sort = RNA_property_enum_get(op->ptr, prop);
     }
-    else {
-      params->sort = U_default.file_space_data.sort_type;
-    }
 
-    if (params->display == FILE_DEFAULTDISPLAY) {
-      params->display = U_default.file_space_data.display_type;
+    if (params->sort == FILE_SORT_DEFAULT) {
+      params->sort = U_default.file_space_data.sort_type;
     }
 
     if (is_relative_path) {
@@ -327,8 +328,9 @@ short ED_fileselect_set_params(SpaceFile *sfile)
   return 1;
 }
 
-/* The subset of FileSelectParams.flag items we store into preferences. */
-#define PARAMS_FLAGS_REMEMBERED (FILE_HIDE_DOT | FILE_SORT_INVERT)
+/* The subset of FileSelectParams.flag items we store into preferences. Note that FILE_SORT_ALPHA
+ * may also be remembered, but only conditionally. */
+#define PARAMS_FLAGS_REMEMBERED (FILE_HIDE_DOT)
 
 void ED_fileselect_window_params_get(const wmWindow *win, int win_size[2], bool *is_maximized)
 {
@@ -341,6 +343,22 @@ void ED_fileselect_window_params_get(const wmWindow *win, int win_size[2], bool
   *is_maximized = WM_window_is_maximized(win);
 }
 
+static bool file_select_use_default_display_type(const SpaceFile *sfile)
+{
+  PropertyRNA *prop;
+  return (sfile->op == NULL) ||
+         !(prop = RNA_struct_find_property(sfile->op->ptr, "display_type")) ||
+         (RNA_property_enum_get(sfile->op->ptr, prop) == FILE_DEFAULTDISPLAY);
+}
+
+static bool file_select_use_default_sort_type(const SpaceFile *sfile)
+{
+  PropertyRNA *prop;
+  return (sfile->op == NULL) ||
+         !(prop = RNA_struct_find_property(sfile->op->ptr, "sort_method")) ||
+         (RNA_property_enum_get(sfile->op->ptr, prop) == FILE_SORT_DEFAULT);
+}
+
 void ED_fileselect_set_params_from_userdef(SpaceFile *sfile)
 {
   wmOperator *op = sfile->op;
@@ -352,12 +370,6 @@ void ED_fileselect_set_params_from_userdef(SpaceFile *sfile)
     return;
   }
 
-  if (!RNA_struct_property_is_set(op->ptr, "display_type")) {
-    sfile->params->display = sfile_udata->display_type;
-  }
-  if (!RNA_struct_property_is_set(op->ptr, "sort_method")) {
-    sfile->params->sort = sfile_udata->sort_type;
-  }
   sfile->params->thumbnail_size = sfile_udata->thumbnail_size;
   sfile->params->details_flags = sfile_udata->details_flags;
   sfile->params->filter_id = sfile_udata->filter_id;
@@ -365,6 +377,16 @@ void ED_fileselect_set_params_from_userdef(SpaceFile *sfile)
   /* Combine flags we take from params with the flags we take from userdef. */
   sfile->params->flag = (sfile->params->flag & ~PARAMS_FLAGS_REMEMBERED) |
                         (sfile_udata->flag & PARAMS_FLAGS_REMEMBERED);
+
+  if (file_select_use_default_display_type(sfile)) {
+    sfile->params->display = sfile_udata->display_type;
+  }
+  if (file_select_use_default_sort_type(sfile)) {
+    sfile->params->sort = sfile_udata->sort_type;
+    /* For the default sorting, also take invert flag from userdef. */
+    sfile->params->flag = (sfile->params->flag & ~FILE_SORT_INVERT) |
+                          (sfile_udata->flag & FILE_SORT_INVERT);
+  }
 }
 
 /**
@@ -381,13 +403,24 @@ void ED_fileselect_params_to_userdef(SpaceFile *sfile,
   UserDef_FileSpaceData *sfile_udata_new = &U.file_space_data;
   UserDef_FileSpaceData sfile_udata_old = U.file_space_data;
 
-  sfile_udata_new->display_type = sfile->params->display;
   sfile_udata_new->thumbnail_size = sfile->params->thumbnail_size;
-  sfile_udata_new->sort_type = sfile->params->sort;
   sfile_udata_new->details_flags = sfile->params->details_flags;
   sfile_udata_new->flag = sfile->params->flag & PARAMS_FLAGS_REMEMBERED;
   sfile_udata_new->filter_id = sfile->params->filter_id;
 
+  /* In some rare cases, operators ask for a specific display or sort type (e.g. chronological
+   * sorting for "Recover Auto Save"). So the settings are optimized for a specific operation.
+   * Don't let that change the userdef memory for more general cases. */
+  if (file_select_use_default_display_type(sfile)) {
+    sfile_udata_new->display_type = sfile->params->display;
+  }
+  if (file_select_use_default_sort_type(sfile)) {
+    sfile_udata_new->sort_type = sfile->params->sort;
+    /* In this case also remember the invert flag. */
+    sfile_udata_new->flag = (sfile_udata_new->flag & ~FILE_SORT_INVERT) |
+                            (sfile->params->flag & FILE_SORT_INVERT);
+  }
+
   if (temp_win_size && !is_maximized) {
     sfile_udata_new->temp_win_sizex = temp_win_size[0];
     sfile_udata_new->temp_win_sizey = temp_win_size[1];