@techee requested changes on this pull request.
Thanks for the patch - looks good to me in general except for some of the notes
below. I personally think that having such a feature in Geany (i.e. multi-tab
Scrilbble backed by Scintilla with all the Geany features) would be nice, but
depends what others think.
I didn't go through all the things in detail and this is just a rough quick
review.
> Includes one small extension to API (new function in main.h) - if i
> understood correctly, this does not require incrementing any versions
Yes, it does, but it can be done one everything is ironed-out.
> + ui_set_statusbar(TRUE, "%s", message);
+ g_warning("%s", message);
+ g_free(message);
+ g_free(new_filename);
+ return;
+ }
+
+ close(fd); /* close the returned file descriptor as we only need the
filename */
+
+ doc->file_name = new_filename;
+
+ if (ft != NULL && ft->id == GEANY_FILETYPES_NONE)
+ document_set_filetype(doc,
filetypes_lookup_by_name(default_ft));
+
+ /* force saving the file to enable all the related actions(tab name,
filetype, etc.) */
+ document_save_file(doc, TRUE);
Couldn't document_save_file_as be used here instead? It should do the filetype
detection for you and shouldn't require the kind of hacky `doc->file_name =
new_filename`. (I know this was copied from the previous code but this might be
better unless I overlook something.)
> +
+ if (file_path == NULL)
+ {
+ return FALSE;
+ }
+
+ filename = g_path_get_basename(file_path);
+ matched = is_temp_saved_file(filename);
+
+ g_free(filename);
+
+ return matched;
+}
+
+
+static void load_all_temp_files_into_editor(const gchar *path)
Is this whole function needed? Couldn't persistent temp files just use the
standard Geany session management so they are stored either in the global
session or a project based session and restored from it automatically by Geany?
> + }
+ else
+ {
+ g_remove(locale_file_path);
+
+ ui_set_statusbar(TRUE, _("Empty temp file %s
was deleted"), short_filename);
+ }
+
+ g_free(short_filename);
+ g_free(locale_file_path);
+ }
+ }
+}
+
+
+static void persistent_temp_files_document_before_save_cb(GObject *obj,
GeanyDocument *doc, gpointer user_data)
I guess you are trying to handle the "save as" case here, right? Then you could
use the newly added `document-before-save-as`:
https://github.com/geany/geany/pull/3572
> @@ -347,8 +706,13 @@ static gboolean on_document_focus_out(GObject *object,
> GeanyEditor *editor,
PluginCallback plugin_callbacks[] =
{
{ "document-new", (GCallback) &instantsave_document_new_cb, FALSE, NULL
},
+ { "document-new", (GCallback) &persistent_temp_files_document_new_cb,
FALSE, NULL },
I'd suggest having a single handler which then dispatches it to either the
instantsave case or the persistent temp file case. Same for `document-save`
below.
> + locale_file_path = g_build_path(G_DIR_SEPARATOR_S, path,
> filename, NULL);
+
+ if (is_temp_saved_file(utf8_filename))
+ {
+ document_open_file(locale_file_path, FALSE, NULL, NULL);
+ }
+
+ g_free(utf8_filename);
+ g_free(locale_file_path);
+ }
+
+ g_dir_close(dir);
+}
+
+
+static gboolean file_exists(gchar *utf8_file_path)
Instead of this function you can just check `doc->real_path != NULL`.
> + }
+}
+
+
+static void persistent_temp_files_document_close_cb(GObject *obj,
GeanyDocument *doc, gpointer user_data)
+{
+ gchar *file_path, *locale_file_path, *short_filename;
+
+ file_path = doc->file_name;
+
+ if (enable_persistent_temp_files && file_path != NULL && !
geany_is_quitting())
+ {
+ if (is_temp_saved_file_doc(doc) && file_exists(doc->file_name))
+ {
+ short_filename = document_get_basename_for_display(doc,
-1);
+ locale_file_path =
utils_get_locale_from_utf8(file_path);
You can use `doc->real_path` instead.
> @@ -462,6 +462,18 @@ gboolean main_is_realized(void)
}
+/**
+ * Checks whether Geany is quitting completely right now.
+ *
+ * @return @c TRUE if the Geany is exiting right now or @c FALSE otherwise.
+ **/
+GEANY_API_SYMBOL
+gboolean geany_is_quitting(void)
I don't think you want `main_status.quitting` here - IMO you want
`main_status.closing_all` instead. Note that the plugin should work both for:
- the default session (list of open files) when no project is open
- project session when some project is open
I think you can just safely rely on Geany's session management and let Geany
store persistent temp files into the corresponding session and reopen them from
there.
You need to use `main_status.closing_all` to detect situations when a session
is being closed including all its files and this can happen either when a
project is closed (for project sessions) or when Geany quits (default session).
> return;
}
- close(fd); /* close the returned file descriptor as we only
need the filename */
+ old_file_name = g_path_get_basename(old_file_path);
+
+ if (is_temp_saved_file(old_file_name) && !
g_str_equal(old_file_path, new_file_path))
+ {
+ /* we have to store old/new filename pair in a global
hashtable to be able to somehow
+ pass it to document-save callback that is called
directly after this one */
+ g_hash_table_insert(
You can use
```C
gpointer plugin_get_document_data(struct GeanyPlugin *plugin,
struct GeanyDocument *doc, const gchar *key);
void plugin_set_document_data(struct GeanyPlugin *plugin, struct GeanyDocument
*doc,
const gchar *key, gpointer data);
```
to assign/retrieve arbitrary data to/from a document which is probably more
elegant in this case.
> +
+ if (file_path == NULL)
+ {
+ return FALSE;
+ }
+
+ filename = g_path_get_basename(file_path);
+ matched = is_temp_saved_file(filename);
+
+ g_free(filename);
+
+ return matched;
+}
+
+
+static void load_all_temp_files_into_editor(const gchar *path)
Seems to work when I comment-out this function call.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3911#pullrequestreview-2130804944
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/3911/review/[email protected]>
