[Geany-Devel] Simplifying file saving options

[Jiří Techet]

On Wed, Aug 19, 2015 at 12:43 PM, Lex Trotman <elextr at gmail.com> wrote:

> On 19 August 2015 at 19:48, Jiří Techet <techet at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.geany.org/pipermail/devel/attachments/20150819/9da7c695/attachment-0001.html>