On Wed, Aug 19, 2015 at 12:43 PM, Lex Trotman <ele...@gmail.com> wrote:
> On 19 August 2015 at 19:48, Jiří Techet <tec...@gmail.com> wrote: > > Hi, > > > > after some time spent debugging > > > > https://github.com/geany/geany/issues/605 > > > > I think the file saving options are a bit confusing and misleading and > > should be simplified. > > Agree the usability is poor with several options that interact with each > other. > > > > > 1. Both g_file_set_contents() and g_file_replace_contents() do absolutely > > the same thing (saving to temporary file, then renaming the file), > > No they are not the same, set_contents does not work on file systems > where rename is not available or is not allowed over existing files > (windows), and it creates a new file with new file metadata (eg > permissions), whereas replace_contents is much more complex than set, > it tries to compensate for these issues but can do several copies of > the data during the attempt, which is very costly on slow remote > filesystems and has a bug (see below). > I meant "same" in the sense they both should be safe. Yes, you are right, g_file_replace_contents() tries to mitigate the problems of g_file_set_contents(). By the way g_file_set_contents() works on Windows, it just isn't atomic: https://github.com/bratsche/glib/blob/master/glib/gfileutils.c#L1087 > > > the only > > difference is the first uses POSIX calls while the second GIO. The > > use_gio_unsafe_file_saving name is very confusing because it's not > unsafe in > > any way. > > It is unsafe due to a long standing bug where a failure deletes the > copy of the original data without copying it back to the original > file, so you are left with a truncated original file and no backup of > the original data. You still have the new data in the Geany buffer of > course. But it regularly has caught people out if they closed the > buffer after a failure assuming they had the original data safe. > Uh, just checked the code and it really seems to be the case. Might fix that if I have time. > > > > > 2. It is now possible to set > > > > use_gio_unsafe_file_saving=true > > use_atomic_file_saving=true > > > > and to users it's unclear which of the options will be actually used > (it's > > the use_atomic_file_saving case because it's checked first in the code). > > Yeah, an enum not a set of bools would be better, IIUC the current > system grew all the options over time rather than being planned. For > sure cleaning it up would be good. > > > > > 3. The fallback "ordinary" file saving when > > > > use_gio_unsafe_file_saving=false > > use_atomic_file_saving=false > > > > doesn't bring any benefit compared to the two above > > It just overwrites the file, nothing else, one data transfer, fast, no > rename, works on *all* filesystems, keeps the files metadata. Whilst > its not in any way "safe" its got the best performance. > OK, might make sense to keep it then. Jiri
_______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel