After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 506350 - GIO port
GIO port
Status: RESOLVED FIXED
Product: gedit
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Gedit maintainers
Gedit maintainers
Depends on:
Blocks:
 
 
Reported: 2007-12-30 00:20 UTC by Steve Frécinaux
Modified: 2009-01-05 13:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] First draft of GeditGIODocumentLoader (14.65 KB, patch)
2007-12-30 00:22 UTC, Steve Frécinaux
none Details | Review
[PATCH] GIO conversion for GeditDocument (48.15 KB, patch)
2008-01-27 20:27 UTC, Steve Frécinaux
none Details | Review
gedit utils (33.53 KB, patch)
2008-02-25 15:44 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
utils patch (1.98 KB, patch)
2008-02-25 15:51 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review
GIO loader/saver (91.65 KB, patch)
2008-08-08 13:40 UTC, jessevdk@gmail.com
none Details | Review
patch (91.26 KB, patch)
2008-08-12 09:15 UTC, Paolo Borelli
none Details | Review
patch (101.48 KB, patch)
2008-08-12 10:30 UTC, Paolo Borelli
none Details | Review
committed patch (104.62 KB, patch)
2008-08-13 11:09 UTC, Paolo Borelli
committed Details | Review

Description Steve Frécinaux 2007-12-30 00:20:39 UTC
This constitutes the first step towards a GIO integration within gedit.
Comment 1 Steve Frécinaux 2007-12-30 00:22:26 UTC
Created attachment 101822 [details] [review]
[PATCH] First draft of GeditGIODocumentLoader


Issues: it seems there is a bug with the code that gets the mtime.
---
 configure.ac                      |    1 +
 gedit/Makefile.am                 |    2 +
 gedit/gedit-document-loader.c     |    8 +-
 gedit/gedit-gio-document-loader.c |  334 +++++++++++++++++++++++++++++++++++++
 gedit/gedit-gio-document-loader.h |   77 +++++++++
 5 files changed, 420 insertions(+), 2 deletions(-)
Comment 2 Steve Frécinaux 2008-01-27 20:27:53 UTC
Created attachment 103837 [details] [review]
[PATCH] GIO conversion for GeditDocument

 gedit/gedit-document-loader.c          |   53 ++++---
 gedit/gedit-document-loader.h          |   18 +--
 gedit/gedit-document-saver.c           |   49 +++---
 gedit/gedit-document-saver.h           |   22 ++--
 gedit/gedit-document.c                 |  288 ++++++++++++++------------------
 gedit/gedit-document.h                 |    4 +-
 gedit/gedit-gio-document-loader.c      |   22 +--
 gedit/gedit-gnomevfs-document-loader.c |   14 +-
 gedit/gedit-gnomevfs-document-saver.c  |   12 +-
 gedit/gedit-local-document-saver.c     |   22 ++--
 gedit/gedit-marshal.list               |    4 +-
 gedit/gedit-metadata-manager.c         |   23 ++-
 gedit/gedit-metadata-manager.h         |    5 +-
 gedit/gedit-mmap-document-loader.c     |   14 +-
 14 files changed, 256 insertions(+), 294 deletions(-)
Comment 3 Ignacio Casal Quinteiro (nacho) 2008-02-25 15:44:49 UTC
Created attachment 105914 [details] [review]
gedit utils

A bit hack to remove gnome vfs from two funcs in gedit-utils
Comment 4 Ignacio Casal Quinteiro (nacho) 2008-02-25 15:51:03 UTC
Created attachment 105915 [details] [review]
utils patch

I don't know what damn i upload before. Sorry about that.
Comment 5 Steve Frécinaux 2008-02-28 08:43:12 UTC
Ignacio: I think you should at least support gnome-open as well as xdg-open, since we are still in the gnome world...

In the long term, what should disappear are all the URIs that are spat throughout gedit. The approach I took in my patches was to change the core while exposing the same API, and change it incrementally. But there is still much work to be done...
Comment 6 Mikael Hermansson 2008-05-05 20:00:43 UTC
I may be wrong but isn't gnome-open in libgnome[ui]? 

if so just cut and paste that bit of code until there is a replacement in GLIB.

Reason:

we really want to KILL libgnome/bonobo deps because that libs is dragging in a hell lot of libs for example gnomevfs and that way we get more dependies than before GVFS was there and that is not what we want :-)



Comment 7 André Klapper 2008-06-05 15:01:55 UTC
so, any news here? do we need a reworked patch, or what is the status?
Comment 8 André Klapper 2008-06-22 16:30:23 UTC
yes, gnome-open IS in libgnome, also see http://live.gnome.org/LibgnomeMustDie#line-34 .

> if so just cut and paste that bit of code until there is a replacement in GLIB

we don't really kill libs by duplicating their code. from my POV we want to get rid of gnome-vfs; libgnome's deps on gnome-vfs is a different issue to deal with.
So please get this in. Somehow. :-)
Comment 9 Andreas Henriksson 2008-07-13 21:04:17 UTC
This seems relevant for the "utils patch":
http://live.gnome.org/GnomeGoals/RemoveGnomeOpenGnomeHelp
(use gtk_show_uri)
Comment 10 jessevdk@gmail.com 2008-08-08 13:40:13 UTC
Created attachment 116143 [details] [review]
GIO loader/saver

Little update, most of the gnomevfs code has now been removed from the general parts of gedit (and plugins) as well as removing gnome_url_show and gnome_show_help (replaced by gtk_show_uri). This is already in the current trunk of gedit. The last parts (the loader, saver, error messages) can be found in the attached patch and need a more thorough review than the other small bits I already committed.
Comment 11 Paolo Borelli 2008-08-09 13:52:47 UTC
First round of comments:

 * strerror -> g_strerror
 * replace the get_mime_type method of saver/loader with get_content_type
 * the loader/saver load() and save() funcs should take a GFile directly (this  however can be refined later incrementally)
 * the warning fix in the open-location-dialog can go in immediately
 * add yourself to authors/copiright of the files :)
 * is get_mount_operation_accumulator still needed?
 * I do not like gedit_document_saver_get_document_contents much, we should try to move away from the need to buffer a whole copy of the content
Comment 12 Paolo Borelli 2008-08-10 08:18:10 UTC
more comments:


+ /* we use GIO error codes, so start at some high number? */

this sounds wrong... why are we just using the error codes? If we get a GError from gio, we should "bubble it up" entirely with it's own domain


The mount op factory function we discussed on irc should probably also take a GFunc to free the user data to be a bit more flexible... I am not sure if in our case it's a good idea to ref/unref the tab. Also please let's add a comment about what this func does and why it is needed.


We use some idle func/timeouts to run the save/load, and this can interact with gtk to use the mount operation: I think the idle func content should be protected by the gdk lock as documented here http://library.gnome.org/devel/gdk/unstable/gdk-Threads.html


Should we use GBufferedOutputStream instead of manually chunking the write ourself in the gio saver? (just an idea, not sure if it is a good idea or even doable)
Comment 13 Paolo Borelli 2008-08-12 09:15:37 UTC
Created attachment 116405 [details] [review]
patch

This is Jesse's patch updated to apply cleanly against head and with some minor problems fixed: in particular when reading an attribute from GFileInfo we cannot rely on the fact that it is there, we have to check with _has_attribute().

Most of the my previous comments still apply.
Comment 14 Paolo Borelli 2008-08-12 10:30:29 UTC
Created attachment 116408 [details] [review]
patch

Same patch as before, but saver/loader refactored to expose content_type and not mime_type (content_type -> mime_type conversion happens in gedit-document for now, later we should also expose content type from the doc and use it directly for the lang-manager)
Comment 15 Paolo Borelli 2008-08-12 13:52:00 UTC
I further tested and reviewed this... I think we should commit and release in order to maximize user testing and work incrementally to fix up the remaining issues. If anyone think this is a bad idea, please shout now :)
Comment 16 Paolo Borelli 2008-08-13 11:09:31 UTC
Created attachment 116492 [details] [review]
committed patch

Ok, no one shouted. This is the patch that was committed.

I am not closing this bug since I think we should still work out some details, for instance the ones in the previus comments. The one about GErrors is the most pressing in my opinion
Comment 17 jessevdk@gmail.com 2008-08-17 09:02:14 UTC
I've just committed a fix that propagates the gio errors properly. Things that need to be done:

* Change gedit internals to use content type, we should only use mime_type for gsv
* Check idle func/timeouts to run the save/load and the gdk lock
* Factor the encoding conversion in a decoration buffer object (GeditEncodingBuffer?). We should also try to not copy the whole text buffer while saving, but incrementally encode and write the text buffer. 

Comment 18 Paolo Borelli 2009-01-05 13:32:37 UTC
I think we can close this now