As discussed in https://github.com/geany/geany/pull/1180.
This mechanism is more consistent in the context of "Saving", and is easier to understand with respect to how it works, so I hope it gets adopted. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1190
-- Commit Summary --
* Change Save As->Rename to Save As->Save and Remove Original
-- File Changes --
M src/dialogs.c (54)
-- Patch Links --
https://github.com/geany/geany/pull/1190.patch https://github.com/geany/geany/pull/1190.diff
g_return_val_if_fail(!EMPTY(utf8_filename), FALSE);
if (doc->file_name != NULL) {
if (rename_file)
{
document_rename_file(doc, utf8_filename);
}
if (remove_orig_file && strcmp(utf8_filename, doc->file_name) != 0)
Is doc->file_name UTF8?
success = document_save_file_as(doc, utf8_filename);
- if (orig_file_name)
- {
if (success)
{
gchar *orig_file_name_locale = utils_get_locale_from_utf8(orig_file_name);
if (g_remove(orig_file_name_locale) != 0)
dialogs_show_msgbox_with_secondary(GTK_MESSAGE_ERROR,
_("Error deleting original file."), g_strerror(errno));
Maybe a more re-assuring "File saved, but original file removal failed" or something similar?
Couple of minor comments, otherwise LGBI.
I have not reviewed the internal semantic change yet and don't much care, as long as it still lets me give my files a new name (and never leaves me without old or new file on disk on error), but I don't like the change to 'Save and Remove Original' for a number of reasons: 1) it obfuscates what the feature does, 'Rename' is perfectly fine no matter the exact implementation details and 'Save and Remove Original' is just a really weird and confusing way to say 'Rename', 2) it causes needless translation churn and may even be hard to translate in some languages, 3) it could cause the dialog button to be even longer than the already too long 'Save and Remove Original' in some languages.
1) it no longer renames, thats stupid, please read my #1180 rant for why. 2) since the implementation is different, it seems reasonable that the label needs to be different, so the change is needed. Given the translation "churn" we do without thought, I doubt this will matter. And I don't see why it should be any harder to translate. 3) Indeed, better suggestions are welcome
- it no longer renames, thats stupid, please read my #1180 rant for why.
If it no longer performs a rename (that is, gives a file a new name), then a whole new feature should be added, the rename feature is extremely useful and shouldn't be dropped.
@codebrainz it does give it a new name, its "save as".
I don't understand.
I mean if the "Rename" button in the "Save As" dialog still results in the current document contents being written to the new filen and the old file no longer existing, it should be left "Rename". If it produces a different outcome in the success case, then there should be a whole new button next to "Rename" that does whatever else.
Hmm, ok, I suppose that, since the effect is the same even if its done in another way, the button could stay "Rename".
If there's important details the user must know about the implementation, we could add them to the manual, or even if really important, to the button's tooltip.
Yeah, probably the tooltip.
Oh shoot, this doesn't produce the same result as the current implementation.
I completely forgot about metadata, with this implementation all files given new names are new files, so they have new file metadata. Owner and permissions may change, but specifically the execute bit won't be carried over from the original file. So shell scripts or Python scripts or similar will no longer run until the user manually re-adds the execute.
With the current implementation the metadata is preserved depending on the type of saving in use, the "safe" file save mode will not preserve the metadata but the others will.
Sorry @konsolebox we probably should stick to the existing implementation.
g_return_val_if_fail(!EMPTY(utf8_filename), FALSE);
if (doc->file_name != NULL) {
if (rename_file)
{
document_rename_file(doc, utf8_filename);
}
if (remove_orig_file && strcmp(utf8_filename, doc->file_name) != 0)
Almost every part of the code treats it as UTF-8 (and converts it sometimes to current locale with `utils_get_locale_from_utf8`; e.g. `gchar *old_locale_filename = utils_get_locale_from_utf8(doc->file_name);`). Or am I wrong?
success = document_save_file_as(doc, utf8_filename);
- if (orig_file_name)
- {
if (success)
{
gchar *orig_file_name_locale = utils_get_locale_from_utf8(orig_file_name);
if (g_remove(orig_file_name_locale) != 0)
dialogs_show_msgbox_with_secondary(GTK_MESSAGE_ERROR,
_("Error deleting original file."), g_strerror(errno));
Yes.
I completely forgot about metadata, with this implementation all files given new names are new files, so they have new file metadata. Owner and permissions may change, but specifically the execute bit won't be carried over from the original file. So shell scripts or Python scripts or similar will no longer run until the user manually re-adds the execute.
That was actually the difference that I was trying to realize back then. Yeah, looks like the feature should remain.
Sorry @konsolebox we probably should stick to the existing implementation.
It's ok, this was just a side-concern and it wasn't difficult to make. I'm actually happy with the ideas I'm hearing about this.
Going back to the original issues, do you still think we shouldn't abort saving changes when a rename fails?
Closed #1190.
Going back to the original issues, do you still think we shouldn't abort saving changes when a rename fails?
Well, since the rename operation is an integral part of the semantics of the "rename" version of "save as", yes it should abort if it can't do it.
Closing in favour of #1180.
github-comments@lists.geany.org