@techee requested changes on this pull request.
Looks promising! Variable name lengths are under control now :-)
I have a few minor comments but apart from them, I think we are close to merging this! 🎉
g_free(next_file_name);
- } else { + } + else + { if (strlen(extension_postfix))
Because of the added `g_strdup("")` above this check should be removed too.
config, "saveactions", "enable_persistent_untitled_documents", FALSE);
+ if (enable_instantsave && enable_persistent_docs) + { + dialogs_show_msgbox(GTK_MESSAGE_ERROR, + _("Invalid config file state: multiple features of 'Persistent Untitled Documents' are enabled at once")); + + enable_instantsave = FALSE; + }
Yes, this is what I had in mind, thanks. I'd just drop the message box - it may just be confusing for users what to do. Since the plugin does a reasonable thing by itself, I think it's OK without the message.
@@ -551,7 +554,7 @@ static void show_unsaved_dialog_for_persistent_untitled_docs_tab_closing(
case GTK_RESPONSE_NO: g_remove(doc->real_path);
- ui_set_statusbar(TRUE, _("Untitled document file %s was deleted"), short_filename); + msgwin_status_add(_("Untitled document file %s was deleted"), short_filename);
Not sure if I made a mistake before or you but I think `ui_set_statusbar` should be used here as this is the destructive action.
g_signal_emit_by_name(pref_widgets.checkbox_enable_backupcopy, "toggled");
+ g_signal_emit_by_name(pref_widgets.untitled_doc_disabled_radio, "toggled"); + g_signal_emit_by_name(pref_widgets.untitled_doc_instantsave_radio, "toggled"); + g_signal_emit_by_name(pref_widgets.untitled_doc_persistent_radio, "toggled");
Should be probably sufficient to emit it for just one of them, right?
g_free(tmp);
+ } + + tmp = utils_get_setting_string(config, "untitled_document_save", "persistent_untitled_documents_target_dir", NULL); + SETPTR(tmp, utils_get_locale_from_utf8(tmp)); + /* Set target dir variable with value from config, regardless if dir is valid or not */ + SETPTR(persistent_docs_target_dir, g_strdup(tmp)); + + if (enable_persistent_docs && ! is_directory_accessible(tmp)) + { + /* switch functionality off, so invalid target directory cannot be actually used */ + enable_persistent_docs = FALSE; + g_key_file_set_boolean(config, "saveactions", "enable_persistent_untitled_documents", FALSE); + + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(pref_widgets.untitled_doc_disabled_radio), TRUE); + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(pref_widgets.untitled_doc_persistent_radio), FALSE);
This line should not be necessary I believe because of the previous line and because it's a radio.
}
+ } else {
`}` and `{` on separate lines
{
- g_key_file_set_string(config, "backupcopy", "backup_dir", backupcopy_text_dir); + /* If target dir path ends with dir separator - we consider it invalid */
Why? If it's a problem somewhere, it can be corrected by the plugin easily: ``` persistent_docs_text_dir[strlen(persistent_docs_text_dir) - 1] = '\0'; ```
if (enable_instantsave)
{ if (EMPTY(instantsave_text_dir)) { - g_key_file_set_string(config, "instantsave", "target_dir", ""); + g_key_file_set_string(config, "untitled_document_save", "instantsave_target_dir", "");
I think the whole plugin would deserve to define section and config names as macros - this is a bit error-prone.
Probably not necessary as part of this PR, I might open a separate PR with this afterwards.
+ new_file_path_utf8 = DOC_FILENAME(doc); + old_file_path_utf8 = plugin_get_document_data(geany_plugin, doc, "file-name-before-save-as"); + + if (old_file_path_utf8 != NULL) + { + if (is_persistent_untitled_doc_file(old_file_path_utf8) + && ! g_str_equal(old_file_path_utf8, new_file_path_utf8)) + { + /* remove untitled doc file if it was saved as some other file */ + gchar *locale_old_file_path = utils_get_locale_from_utf8(old_file_path_utf8); + g_remove(locale_old_file_path); + + g_free(locale_old_file_path); + + ui_set_statusbar(TRUE, _("Untitled document file %s was deleted"), old_file_path_utf8);
Yes, it was you who made a mistake :-). Compared to the case above, `msgwin_status_add` should be used here.