Based on the discussion here https://github.com/geany/geany/issues/921 I tried what is still missing to support opening remote files without gvfs-fuse. As I suspected, there wasn't much missing - mostly some g_file_test() calls which didn't work remotely, file name validation which didn't allow URIs, getting filenames instead of URIs from file dialogs etc.
What works after these patches: * opening links like https://developer.gnome.org/gio/stable/GFile.html both from command-line and file open dialog * browsing remote locations in file browser * opening/saving ftp/ssh/etc. locations when mounted - I guess they need to be mounted to work - even gedit needs them mounted (it would probably be possible to use g_file_mount_enclosing_volume () to pop up dialog to make user mount them, not sure if we want it or not) * opening/saving remote projects
What doesn't work: * TM parsing - the only reason here's the g_stat() in tm_source_file_init(), after commenting it out it works but TM at the moment doesn't know about Geany's file operation method used so it's not possible to implement this properly (I plan moving the TM - without ctags - into src making it effectively part of Geany after which it will have access to Geany's definitions) * File->Properties - I simply have been lazy to implement it * of course things like find in files, build commands etc. which we cannot run remotely You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/963
-- Commit Summary --
* Add wrappers around g_file_test() and use them everywhere * Update utils_get_path_from_uri() to return URI when using GIO * Fix warning in utils_tidy_path() * Use g_file_new_for_uri() when needed * Preserve URIs in tm_get_real_path() * Merge implementations of write_data_to_disk() and utils_write_file() * Use gtk_file_chooser_get_uris() in file opening/saving dialogs * Add GIO version of document_rename_file() * Add GIO implementation to utils_get_file_list_full() * Add file reading utils function * Load project config files using the new utility function * Allow opening projects using URI
-- File Changes --
M plugins/export.c (2) M plugins/filebrowser.c (6) M plugins/htmlchars.c (2) M plugins/saveactions.c (6) M src/build.c (4) M src/callbacks.c (8) M src/dialogs.c (8) M src/document.c (131) M src/editor.c (2) M src/highlighting.c (6) M src/keybindings.c (4) M src/keyfile.c (4) M src/libmain.c (24) M src/msgwindow.c (6) M src/plugins.c (2) M src/project.c (57) M src/symbols.c (2) M src/templates.c (2) M src/ui_utils.c (6) M src/utils.c (322) M src/utils.h (16) M src/vte.c (2) M src/win32.c (4) M tagmanager/src/tm_source_file.c (16)
-- Patch Links --
https://github.com/geany/geany/pull/963.patch https://github.com/geany/geany/pull/963.diff
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963
@techee nice!!!
Two questions:
1. what does the file dialog show and what do we get for IRIs, a pretty UTF-8 version or the uglyfied punycode or percent encoded version?
2. Does the save-actions plugin need adjusting too?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-196093112
what does the file dialog show and what do we get for IRIs, a pretty UTF-8 version or the uglyfied punycode or percent encoded version?
Could you provide an example of such an URI pointing to some text file so I can try opening it in Geany?
Does the save-actions plugin need adjusting too?
From what I can see the backup saving code would have to be updated to work remotely.
[Edit: 3. when the settings are not set to GIO does this mix GIO and non-GIO calls? (remembering that gave us problems with stat in the past) ]
Since I fixed this one I took care not to mix GIO/non-GIO for files which could be opened remotely. Now with local files like various Geany config files it shouldn't matter and these typically still use the non-GIO variants all the time. The exception will be the various g_file_test() calls I replaced in the first patch and which might use GIO on local files . There are quite many of them and I wanted to avoid evaluating every single case whether it needs GIO or not. But again, since they are local and since they have nothing to do with file updates there's no danger there would be some kind of race like there was in the mtime case.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-196318468
@@ -2188,3 +2188,91 @@ void utils_start_new_geany_instance(const gchar *doc_path) else g_printerr("Unable to find 'geany'"); }
+static GFile *utils_gfile_create(const gchar *fname) +{
- if (utils_is_uri(fname))
return g_file_new_for_uri(fname);- return g_file_new_for_path(fname);
+}
+/**
- Tests file existence using the configured IO method in Geany.
- @param fname File name.
"the path or URI"
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/4d2f2045b35231c944904edd5540bf...
@@ -2188,3 +2188,91 @@ void utils_start_new_geany_instance(const gchar *doc_path) else g_printerr("Unable to find 'geany'"); }
+static GFile *utils_gfile_create(const gchar *fname) +{
- if (utils_is_uri(fname))
return g_file_new_for_uri(fname);- return g_file_new_for_path(fname);
+}
+/**
- Tests file existence using the configured IO method in Geany.
- @param fname File name.
Do we actually want those in the API? It's cute, but do plugin really care about that?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/4d2f2045b35231c944904edd5540bf...
Could you provide an example of such an URI pointing to some text file so I can try opening it in Geany?
No, I only speak ASCII remember :) You or @b4n would be more likely to know of a site with an accented character in the domain name.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-198803442
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename) const gchar *needle; gboolean preserve_double_backslash = FALSE;
- if (utils_is_uri(filename))
return;
what about skipping the scheme instead? around the line of a sane version of `filename = strstr(filename, "://") + 3`.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/f366d1d40972ffbe7c7216301f8582...
@@ -334,6 +334,8 @@ const gchar *utils_resource_dir(GeanyResourceDirType type);
void utils_start_new_geany_instance(const gchar *doc_path);
+GFile *utils_gfile_create(const gchar *fname);
should stay private or get documented
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/b4cf42397c77249d18d5972e981e9e...
@@ -334,6 +334,8 @@ const gchar *utils_resource_dir(GeanyResourceDirType type);
void utils_start_new_geany_instance(const gchar *doc_path);
+GFile *utils_gfile_create(const gchar *fname);
oops, scratch that, misread the patch it's private
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/b4cf42397c77249d18d5972e981e9e...
@@ -1034,12 +1034,15 @@ static gboolean load_config(const gchar *filename) GKeyFile *config; GeanyProject *p; GSList *node;
- gchar *data;
- gsize len;
needs freeing
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/f2689a096672955ba1dfbcec3f5247...
@@ -1110,7 +1114,8 @@ static gboolean write_config(gboolean emit_signal) config = g_key_file_new(); /* try to load an existing config to keep manually added comments */ filename = utils_get_locale_from_utf8(p->file_name);
- g_key_file_load_from_file(config, filename, G_KEY_FILE_NONE, NULL);
- if (utils_read_file(filename, &data, &len, NULL))
needs freeing
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/f2689a096672955ba1dfbcec3f5247...
@@ -343,7 +343,8 @@ void project_open(void) gtk_window_set_skip_taskbar_hint(GTK_WINDOW(dialog), TRUE); gtk_window_set_type_hint(GTK_WINDOW(dialog), GDK_WINDOW_TYPE_HINT_DIALOG); gtk_window_set_transient_for(GTK_WINDOW(dialog), GTK_WINDOW(main_widgets.window));
gtk_file_chooser_set_select_multiple(GTK_FILE_CHOOSER(dialog), TRUE);
gtk_file_chooser_set_select_multiple(GTK_FILE_CHOOSER(dialog), FALSE);
hum, what was that?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/a781ffe57ea21cffdda8959c1b690c...
`locale_filename` now can be an URI. Is it's encoding still good? Does all the code handle it being an URI rather than a path gracefully?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-198804628
Looks mostly sensible. But all this `GFile` creation, and this making Geany use GIO more and more makes me wonder: don't we rather want to have a `GFile` member attached to a document in addition to the path, and have this `NULL` when not using GIO? That would re-use the same link all the way, and possibly might prevent some oddies when loading from an URI and writing to a path (when *gvfs-fuse* is available) -- I doubt it should be a problem, but still, could be suboptimal I guess.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-198804732
@elextr
what does the file dialog show and what do we get for IRIs, a pretty UTF-8 version or the uglyfied punycode or percent encoded version?
That's most likely not up to us, and if it doesn't work now it'll probably get fixed in GIO at some point. And GEdit seems to display it just fine when I try to open `http://b%C3%BCcher.ch/%60 (no, I don't have a better [example](https://fr.wikipedia.org/wiki/Punycode) than [the WP one](https://en.wikipedia.org/wiki/Punycode))
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-198805539
@b4n, agree its not up to us, I just wanted to know what the open dialog would show.
And what would show for the filename on the tab and in its tooltip? And in the documents sidebar? And in filebrowser plugin?
I will bet there will be confused users if some things show Unicoded names and some show punycode and percent escaped names :)
Note: both saveactions and filebrowser are core plugins and should be fixed (if they need to be) as part of this change.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-198806214
@@ -2188,3 +2188,91 @@ void utils_start_new_geany_instance(const gchar *doc_path) else g_printerr("Unable to find 'geany'"); }
+static GFile *utils_gfile_create(const gchar *fname) +{
- if (utils_is_uri(fname))
return g_file_new_for_uri(fname);- return g_file_new_for_path(fname);
+}
+/**
- Tests file existence using the configured IO method in Geany.
- @param fname File name.
When you have a look at the plugins the patch affects, they do (maybe not all the g_file_test()s deal with possibly remote files but some definitely do).
But the question is good - we probably shouldn't make an utility function for every single IO function - instead there should be something like
utils_use_gio()
and plugins should use it as needed (we'll already need this for the saveaction plugin which needs to be updated to make a backup copy of a possibly remote file). This will lead to a minor code duplication in the plugins but I think from the API perspective this is the right way. What do you think?
The other question is whether to make such a function inside utils or somewhere else (prefs?) or whether to make it e.g. a variable in some config struct. Ideas?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/4d2f2045b35231c944904edd5540bf...
@@ -343,7 +343,8 @@ void project_open(void) gtk_window_set_skip_taskbar_hint(GTK_WINDOW(dialog), TRUE); gtk_window_set_type_hint(GTK_WINDOW(dialog), GDK_WINDOW_TYPE_HINT_DIALOG); gtk_window_set_transient_for(GTK_WINDOW(dialog), GTK_WINDOW(main_widgets.window));
gtk_file_chooser_set_select_multiple(GTK_FILE_CHOOSER(dialog), TRUE);
gtk_file_chooser_set_select_multiple(GTK_FILE_CHOOSER(dialog), FALSE);
Copy/paste from File->Open I guess.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/a781ffe57ea21cffdda8959c1b690c...
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename) const gchar *needle; gboolean preserve_double_backslash = FALSE;
- if (utils_is_uri(filename))
return;
Can do. But can relative paths be part of a valid URI?
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963/files/f366d1d40972ffbe7c7216301f8582...
@elextr Works fine as far as I can tell (once it works at one place, it should work everywhere - the rest is just a matter of correct locale/utf8 handling we should do anyway):
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-199014705
locale_filename now can be an URI. Is it's encoding still good? Does all the code handle it being an URI rather than a path gracefully?
Seems so for the core functionality - can't speak for all the plugins. The only exception at the moment is the tag manager which skips parsing because of the g_stat() I mentioned previously.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-199016102
Looks mostly sensible. But all this GFile creation, and this making Geany use GIO more and more makes me wonder: don't we rather want to have a GFile member attached to a document in addition to the path, and have this NULL when not using GIO?
Good idea. I'd just make one change - always create the GFile no matter whether GIO is enabled or not. The problem is GIO can be enabled/disabled in preferences at any time so we'd have to nullify/set the field based on the settings, plugins would have to always check it for NULL and it would introduce just complications. Better to have something like the utils_use_gio() and whoever wants to use the GFile should use the utils_use_gio() or know what he is doing.
That would re-use the same link all the way, and possibly might prevent some oddies when loading from an URI and writing to a path (when gvfs-fuse is available) -- I doubt it should be a problem, but still, could be suboptimal I guess.
I hope when using GIO consistently this shouldn't happen. Anyway, the problem with mtime we had was quite unique I believe and shouldn't happen when e.g. just testing file existence.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-199018501
Note: both saveactions and filebrowser are core plugins and should be fixed (if they need to be) as part of this change.
Agree. Filebrowser already works, saveactions need an update to be able to save backups of remote files.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/963#issuecomment-199020875
elextr commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
As I found out elsewhere not all schemes need the `//` so maybe not such a good idea.
Looks mostly sensible. But all this GFile creation, and this making Geany use GIO more and more makes me wonder: don't we rather want to have a GFile member attached to a document in addition to the path, and have this NULL when not using GIO?
Good idea. I'd just make one change - always create the GFile no matter whether GIO is enabled or not. The problem is GIO can be enabled/disabled in preferences at any time so we'd have to nullify/set the field based on the settings, plugins would have to always check it for NULL and it would introduce just complications. Better to have something like the utils_use_gio() and whoever wants to use the GFile should use the utils_use_gio() or know what he is doing.
I was thinking about it more and it won't eliminate most of the GFile creation. Most of the cases when GFile is created isn't for the already opened document, but when opening a file and e.g. validating it exists. So I didn't make this change in #1414.
techee commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
@b4n I kept it this way in the new patch - the current implementation of the function would have to be changed because there's e.g.
``` utils_string_replace_all(str, "/", G_DIR_SEPARATOR_S) ```
on Windows which we don't want. And since URIs cannot be relative, I think it's more or less OK the way it is?
techee commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
@elextr `://` is used on other places to detect URIs - is there a better way?
elextr commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
@techee I was almost going to say, its ok, for URLs from schemes that are likely to be used as files the `//` is present, but on checking I found that RFC 8089 was released this month for `file:` where one difference from the previous is "The syntax given in Section 2 makes the entire authority component, including the double slashes "//", optional.". So I would say the rule is now starts with `file:` or contains `://`.
Yes its a less likely URL, but people dragging URLs onto Geany might get one.
techee commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
Omitting "//" for URI is rather crazy and I don't expect operating systems or applications will ever pass uris this way. GTK for instance always returns "file://" when opening files and asking for URI. IMO the "//"-less case can be safely ignored.
elextr commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
@techee did you notice the release date THIS MONTH, of course nothing omits it at the moment, its only just been allowed.
elextr commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
Though actually my exhaustive test with Chrome and Firefox shows that browsers are perfectly happy without the // already.
techee commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
@techee did you notice the release date THIS MONTH
Sure, I'm aware of the release and the patches are definitely not meant for it. It's just that I had time now and the bigger pull requests are better to be merged early in the next release cycle to get better testing (of course if Colomban has time to review them).
elextr commented on this pull request.
@@ -1788,6 +1788,9 @@ void utils_tidy_path(gchar *filename)
const gchar *needle; gboolean preserve_double_backslash = FALSE;
+ if (utils_is_uri(filename)) + return;
Sure, I'm aware of the release and the patches are definitely not meant for it.
I didn't mean the release of Geany, I meant the release of RFC 8089 that allows for `file:` to not have the `//`es. As it was only released Feb 2017 nothing will take advantage of it yet, though as I noted both chrome and firefox accept `file:` URLs without the `//`.
github-comments@lists.geany.org