Re: patch-ahoy: gnome_url_show stuff

[Frank Worsley <fworsley shaw ca>]

Hi Alex,

> In general I'm uncertain of the usefulness of the component stuff. If
> you're launching a uri from something that doesn't support component
> embedding itself I think you almost always prefer an application to open
> over a component in a generic shell (do we even have a usable shell for
> this?). I guess if a component is all you have you'd prefer to use that
> over nothing though.

My reasoning is that (for consistency) if you have a component associated as
the default action then the file should always open in the component, no
matter what.

This can be especially important if somebody writes a general component
viewer application (as opposed to just using Nautilus) in the future which
may make components feel more like applications or something like that.

Also important for the case where there is only a component and no
application. Only example I can think off is the RPM view, I haven't tried
it but I don't think it comes with an application.

However, I could also see how this would be really annoying for people who
just want component's for quick previewing. So I'm not sure.

> Also, I'm not sure how this will interact with the work on the mime system
> jrb is working on. We need to make sure to not introduce any new stuff
> that uses things which may become deprecated soon.

It would have been nice if you told me that when I originally posted about
this. I hope all this work wasn't for nothing.

> In expand_parameters():
>
> + if (!app->can_open_multiple_files && uri_list != NULL) {
> + break;
> + }
>
> That needs to be !can_open_multiple_files && uris != NULL

Fixed.

> And in general I think it might be nicer if the command line generation
> stuff worked on argvs instead of commandlines that are escaped and then
> parsed+unescaped at the end.

I see what you mean. That would be a lot cleverer, but also more work. I
will work on it.

> The handling of %s and %c is also a bit
> strange. Typically when you introduce something like that you also allow
> escapes if you need the escape sequences in the string.

If I do the above then that issue will go away in the process.

> +gnome_vfs_use_handler_for_scheme (const char *scheme)
> +gnome_vfs_url_show_using_handler (const char *url)
> +gnome_vfs_url_show_using_handler_with_env (const char *url,
> +                    char      **envp)
>
> These should probably be internal functions. I don't think they are much
> use in the public API.

I originally had them private and then made them public. They could be
useful in some cases.
I will make them private again.

> + desktop_default_applications.schemas
>
> This is not on the site.

Just adds schemas for component_viewer and terminal and a trash: url
handler.

> +gnome_vfs_uri_new_with_unsupported (const gchar *text_uri)
>
> This is some pretty strange API, and a bit badly named to (*with*
> unsupported?). Given the usage of it I think a function to extract the
> schema from a text-uri would be better.

Yes I couldn't think of a better name. Isn't it better to reuse the
GnomeVFSURI code than to make another new function?

> +gnome_url_get_from_input()
>
> The way this function assumes any ':' in the filename means its an
> uri makes it fail on anything with a colon. We should be able to do much
> better than that. And unless i misread the code absolute filenames are
> broken.

Yeah that function is very weak. I will improve it.

> I'm not sure about the API guarantees we have for libgnome. Is extending
> enums ok? Anyway, the value for GNOME_URL_ERROR_PARSE which breaks the
> ABI. At least thats not ok.

Switched that PARSE back to the top. No idea for the extension stuff.

> Why are you removing all the schemas stuff? Is that in the missing
> desktop_default_applications.schemas file?

Removing start-here, application, etc. is ok since they are of type
'x-directory/vfolder-desktop' and Nautilus is associated with that type
anyway. So there is no need for a URL handler. The trash: url handler moved
to desktop_default_applications.schemas. A handler for trash: is still
needed because gnome-vfs thinks its mime type is NULL. I guess trash: is
internal to Nautilus?

> eel.patch.1:
> I don't think these changes are necessary. The way you use them in
> nautilus seems wrong to me. Why would some part of the wait not count
> towards opening the cancel dialog?

Because there is two callbacks we are waiting for and the dialog may pop up
while we are in the first callback. So if you hit cancel then nothing is
going to happen.

But actually, I guess that can't happen because it wont interrupt the
running code to handle the timeout, right?

> +void nautilus_launch_show_file (NautilusFile *file,
> +                                GtkWindow *parent_window)
> +void nautilus_launch_action (GnomeVFSMimeAction *action,
> +                             NautilusFile *file,
> +      GtkWindow *parent_window)
>
> Some indentation issues like these. The return type go on the line before
> the functionname.

Fixed.

> Why would you ever want to not view the component in nautilus? I thought
> the main idea for having components in the mime data was that then a
> component-supporting browser could show these inline?

Maybe because you want to use a special component viewer with a different
interface or other features? I am not sure, but I thought it would be a good
idea to provide that flexibility.

- Frank