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 712620 - Make desktop support optional
Make desktop support optional
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: Desktop
unspecified
Other All
: Normal normal
: 3.22
Assigned To: Nautilus Maintainers
Nautilus Maintainers
: 745284 (view as bug list)
Depends on:
Blocks: wayland 748347
 
 
Reported: 2013-11-18 17:32 UTC by Emilio Pozuelo Monfort
Modified: 2016-04-19 14:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make desktop support optional (18.25 KB, patch)
2013-11-18 17:32 UTC, Emilio Pozuelo Monfort
rejected Details | Review
application: allow overriding open_location_full (3.25 KB, patch)
2016-04-14 08:35 UTC, Carlos Soriano
committed Details | Review
application: add common startup code (3.62 KB, patch)
2016-04-14 08:35 UTC, Carlos Soriano
committed Details | Review
desktop: move to a different binary (19.30 KB, patch)
2016-04-14 08:35 UTC, Carlos Soriano
committed Details | Review
desktop-application: use dbus for opening files (5.85 KB, patch)
2016-04-14 08:35 UTC, Carlos Soriano
committed Details | Review
nautilus-dnd: request file system info if not available (7.89 KB, patch)
2016-04-14 08:35 UTC, Carlos Soriano
committed Details | Review
files-view: remove unused merge/unmerge menus vfunc (2.52 KB, patch)
2016-04-14 08:35 UTC, Carlos Soriano
committed Details | Review
files-view: remove unused can_rename_file vfunc (4.49 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: remove unused is_read_only vfunc (3.00 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: remove unused get_icon_locations vfunc (3.48 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: use inheritance for showing the properties menu item (3.53 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: use inheritance for get_backing_uri (3.97 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: use inheritance for empty states (5.69 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: hide properties menu item when action disabled (1006 bytes, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: use inheritance for querying special selection (6.42 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
files-view: use inheritance for scripts vars (10.28 KB, patch)
2016-04-14 08:36 UTC, Carlos Soriano
committed Details | Review
window-slot: make it derivable class (118.21 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window: allow to create custom slots in subclasses (4.11 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
desktop-window: create custom slot (5.33 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window-slot: allow overriding the creation of views (3.15 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
desktop-window-slot: override creation of views (3.29 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window-slot: use action state instead of special casing the search (3.26 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window-slot: remove unneeded code special casing desktop (1.72 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window-slot: use inheritance for other locations view (21.24 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window-slot: use inheritance for disabling actions (2.43 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
window-slot: remove all dependencies to desktop (1.43 KB, patch)
2016-04-14 08:37 UTC, Carlos Soriano
committed Details | Review
file, directory: move file creation dispatching to directory (5.80 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
directory: allow overriding the creation of files (3.37 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
desktop-directory: override creation of files (3.35 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
desktop: ensure desktop directory on application init (4.97 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
file: allow overriding rename handling (3.32 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
desktop-icon-file, file: move rename handling to subclass (6.14 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
file: allow overriding of get_drop_target_uri (2.06 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
file: rename get_drop_target_uri (4.60 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
desktop-icon-file: override get_target_uri (2.87 KB, patch)
2016-04-14 08:38 UTC, Carlos Soriano
committed Details | Review
file-dnd: move dnd handling to file (10.91 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
file: allow overriding dnd handling (2.48 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
desktop-icon-file: move dnd handling to the subclass (2.01 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
file: allow overriding invalidate_attributes_internal (2.48 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
desktop-icon-file, file: override invalidate_attributes (2.63 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
file-operations: remove obsoleted desktop dependency (925 bytes, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
program-choosing: remove unneeded desktop dependency (882 bytes, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
window: use disable-chrome instead of desktop casting (1.95 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
mime-actions: move opens_in_view to file (7.02 KB, patch)
2016-04-14 08:39 UTC, Carlos Soriano
committed Details | Review
toolbar: use widget visibility instead of desktop special casing (1.98 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
canvas-view: remove unneeded desktop dependency (1.07 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
other-locations-window-slot: remove unneeded desktop dependency (806 bytes, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
properties-window: use get_target_uri instead of custom code (2.42 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
properties-window: use app info for "Open With" visibility (2.26 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
canvas-view: allow creation of canvas view container subclasses (3.21 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
canvas-view-container: provide autoptr cleanup (1.05 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view: override canvas container creation (5.93 KB, patch)
2016-04-14 08:40 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view-container: override get icon description (2.71 KB, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view-container: override get icon text (3.64 KB, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view-container: override compare icons (9.13 KB, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
main-desktop: remove unneeded dependencies (643 bytes, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-application: chain up in dispose (857 bytes, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-application: remove unused priv data (821 bytes, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-application: use g_clear_error (1.95 KB, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-application: create proxy synchronously (2.71 KB, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
canvas-dnd: set as NULL after free (903 bytes, patch)
2016-04-14 08:41 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view: remove unneeded check (1.19 KB, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view: move comment inside the function (814 bytes, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
files-view: fix typo (1.07 KB, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
files-view: move comment inside the function (1013 bytes, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
file: move special link to file instead of the view (9.11 KB, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
files-view: use target_uri for script vars (7.44 KB, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
window-slot: fix formatting (962 bytes, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
window: fix file leak (1.32 KB, patch)
2016-04-14 08:42 UTC, Carlos Soriano
committed Details | Review
window: fix typo (938 bytes, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
window: fix formatting (1.10 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
window: use target_slot, not active slot (1.06 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
window: use helper for replacing slot (3.09 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
file: extract shared code for handling gone files on renaming (4.89 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
desktop-icon-file: chain up for get_target_uri (1.05 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
desktop-icon-file: fix missing parameter in vfunc (1.02 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
toolbar: use gtk_widget_is_visible for robustness (1.11 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view: fix wording (1.27 KB, patch)
2016-04-14 08:43 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view-container: fix formatting (1.22 KB, patch)
2016-04-14 08:44 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view-container: chain up to the parent (1.28 KB, patch)
2016-04-14 08:44 UTC, Carlos Soriano
committed Details | Review
desktop-canvas-view-container: chain up to the parent (1.90 KB, patch)
2016-04-14 08:44 UTC, Carlos Soriano
committed Details | Review
directory: use gio extensions for directory dispatching (15.56 KB, patch)
2016-04-14 08:44 UTC, Carlos Soriano
committed Details | Review
file-dnd: remove the files (6.16 KB, patch)
2016-04-14 08:44 UTC, Carlos Soriano
committed Details | Review
desktop: move to a different folder (29.26 KB, patch)
2016-04-15 18:12 UTC, Carlos Soriano
committed Details | Review
desktop: add a compile flag for building it (3.38 KB, patch)
2016-04-19 14:51 UTC, Carlos Soriano
committed Details | Review

Description Emilio Pozuelo Monfort 2013-11-18 17:32:21 UTC
This makes x11 optional as well.
Comment 1 Emilio Pozuelo Monfort 2013-11-18 17:32:24 UTC
Created attachment 260148 [details] [review]
Make desktop support optional
Comment 2 William Jon McCann 2013-11-18 17:41:03 UTC
Not sure how Cosimo feels but I'm very much in favor of this in general. However, not sure about the approach. Might be better to move the desktop handling into a separate binary and only conditionally build that "service". That way we can separate the app and service part of the project better. And hopefully avoid having as many ifdefs in the code.
Comment 3 Cosimo Cecchi 2013-11-18 22:03:27 UTC
Hi Emilio, thanks for the patch.
I'm very much in favor of this too, but I agree with the comment from Jon above about the approach.

The ideal solution is unfortunately not as trivial. I think a good way forward would be to:
* build two binaries: nautilus and nautilus-desktop, with the latter to entirely handle the desktop alone.
* make it so the desktop-specific C subclasses from src/ are only brought in by nautilus-desktop, possibly move them in a subdirectory of src/.
* either split the common files from src/ into a private library, brought in by both nautilus and nautilus-desktop, or add them to the sources for both nautilus and nautilus-desktop in src/Makefile.am.
* create a separate NautilusDesktopApplication class for nautilus-desktop, and another file containing the main().
* change the ifdefs you put here to either X11-runtime checks (e.g. for the XIDs), or make them virtual methods/properties implemented by NautilusWindow.

How does that sound? Do you think you would have time to work on something like this?
Comment 4 Cosimo Cecchi 2013-11-18 22:05:33 UTC
Review of attachment 260148 [details] [review]:

::: eel/eel-gdk-extensions.c
@@ +52,3 @@
 	g_return_val_if_fail (height_return != NULL, EEL_GDK_NO_VALUE);
 
+/* FIXME: surely we don't need X11 to parse a simple string... */

We might be able to use gtk_window_parse_geometry() here?
Comment 5 Matthias Clasen 2013-11-20 00:54:36 UTC
Review of attachment 260148 [details] [review]:

Overall, I don't like the patch - too much #ifdef

::: src/nautilus-window.c
@@ +761,3 @@
+	 * if desktop support has been disabled. */
+	gint xid = (gint) gdk_x11_window_get_xid (gtk_widget_get_window (GTK_WIDGET (window)));;
+	gchar *xid_string = g_strdup_printf ("%d", xid);

I'd rather make this a runtime check:

#ifdef GDK_WINDOWING_X11
  if (GDK_IS_X11_DISPLAY (...))
    xid = ...
#endif
Comment 6 Emilio Pozuelo Monfort 2013-11-20 16:15:48 UTC
(In reply to comment #4)
> Review of attachment 260148 [details] [review]:
> 
> ::: eel/eel-gdk-extensions.c
> @@ +52,3 @@
>      g_return_val_if_fail (height_return != NULL, EEL_GDK_NO_VALUE);
> 
> +/* FIXME: surely we don't need X11 to parse a simple string... */
> 
> We might be able to use gtk_window_parse_geometry() here?

I knew there had to be something!

(In reply to comment #5)
> Review of attachment 260148 [details] [review]:
> 
> Overall, I don't like the patch - too much #ifdef
> 
> ::: src/nautilus-window.c
> @@ +761,3 @@
> +     * if desktop support has been disabled. */
> +    gint xid = (gint) gdk_x11_window_get_xid (gtk_widget_get_window
> (GTK_WIDGET (window)));;
> +    gchar *xid_string = g_strdup_printf ("%d", xid);
> 
> I'd rather make this a runtime check:
> 
> #ifdef GDK_WINDOWING_X11
>   if (GDK_IS_X11_DISPLAY (...))
>     xid = ...
> #endif

Absolutely, that makes much more sense.

(In reply to comment #3)
> Hi Emilio, thanks for the patch.
> I'm very much in favor of this too, but I agree with the comment from Jon above
> about the approach.
> 
> The ideal solution is unfortunately not as trivial. I think a good way forward
> would be to:
> * build two binaries: nautilus and nautilus-desktop, with the latter to
> entirely handle the desktop alone.
> * make it so the desktop-specific C subclasses from src/ are only brought in by
> nautilus-desktop, possibly move them in a subdirectory of src/.
> * either split the common files from src/ into a private library, brought in by
> both nautilus and nautilus-desktop, or add them to the sources for both
> nautilus and nautilus-desktop in src/Makefile.am.
> * create a separate NautilusDesktopApplication class for nautilus-desktop, and
> another file containing the main().
> * change the ifdefs you put here to either X11-runtime checks (e.g. for the
> XIDs), or make them virtual methods/properties implemented by NautilusWindow.
> 
> How does that sound? Do you think you would have time to work on something like
> this?

I can update the patch for the other comments, but I'm not sure if/when I'll have the time to do that splitting work.
Comment 7 Cosimo Cecchi 2015-02-27 17:31:00 UTC
*** Bug 745284 has been marked as a duplicate of this bug. ***
Comment 8 Carlos Soriano 2016-02-29 15:04:21 UTC
-> this is something we want for 3.22
Comment 9 Carlos Soriano 2016-02-29 15:07:07 UTC
Review of attachment 260148 [details] [review]:

Thanks Emilio for the patch, I will take into account for the new implementation.
I'm rejecting this one since what we want is two different binaries and only the desktop part doing x11 calls.
So implementation will be essentially different as this one.
Comment 10 Carlos Soriano 2016-04-14 08:35:23 UTC
Created attachment 325899 [details] [review]
application: allow overriding open_location_full

So it can work over inheritance. This will be needed for the desktop
handling.
Comment 11 Carlos Soriano 2016-04-14 08:35:34 UTC
Created attachment 325900 [details] [review]
application: add common startup code

So children of nautilus application can chain up to it.
We leave out the parts that children won't be interested in, like
the bus manager which only nautilus is owner.
So far is only used for the opened locations, so it's fine.
Comment 12 Carlos Soriano 2016-04-14 08:35:40 UTC
Created attachment 325901 [details] [review]
desktop: move to a different binary

We wanted to do this for long time. This will allow to handle the
desktop process in a different binary.
The ultimate goal is to make the desktop code completely split from
nautilus code.

This is the first and minimal step towards that goal.

In this patch we create a desktop application separated from nautilus
application, and remove the desktop handling in nautilus application.
Comment 13 Carlos Soriano 2016-04-14 08:35:46 UTC
Created attachment 325902 [details] [review]
desktop-application: use dbus for opening files

Since the desktop is in a different binary now, we need a way to comunicate
the desktop proccess with Nautilus proccess, since we want to avoid
any Nautilus handling in the desktop proccess.

To do so, use the freedesktop dbus API for accessing the file manager.
Comment 14 Carlos Soriano 2016-04-14 08:35:52 UTC
Created attachment 325903 [details] [review]
nautilus-dnd: request file system info if not available

We were assuming we had in cache the files used for dnd, but with
the desktop split that's not longer the case.

What can happens is that we cannot discern whether the drop action
should be a copy or a move, since Nautilus always performs a move if
the file system is the same as the dragged files.

To fix it, in a best effort mode, we will request to update the files
information if it's not available. In future times it will be requested,
and if the files are not freed, we will have the file system information
available in order to decide a more appropriate drop action.
Comment 15 Carlos Soriano 2016-04-14 08:35:58 UTC
Created attachment 325904 [details] [review]
files-view: remove unused merge/unmerge menus vfunc
Comment 16 Carlos Soriano 2016-04-14 08:36:04 UTC
Created attachment 325905 [details] [review]
files-view: remove unused can_rename_file vfunc
Comment 17 Carlos Soriano 2016-04-14 08:36:10 UTC
Created attachment 325906 [details] [review]
files-view: remove unused is_read_only vfunc
Comment 18 Carlos Soriano 2016-04-14 08:36:16 UTC
Created attachment 325907 [details] [review]
files-view: remove unused get_icon_locations vfunc
Comment 19 Carlos Soriano 2016-04-14 08:36:22 UTC
Created attachment 325908 [details] [review]
files-view: use inheritance for showing the properties menu item

So we can remove another special casing of the desktop in a parent class
Comment 20 Carlos Soriano 2016-04-14 08:36:28 UTC
Created attachment 325909 [details] [review]
files-view: use inheritance for get_backing_uri

So we remove another desktop special case
Comment 21 Carlos Soriano 2016-04-14 08:36:35 UTC
Created attachment 325910 [details] [review]
files-view: use inheritance for empty states

We don't want always empty states, for example, in the desktop.
Use inheritance to allow children to do its own handling.
Comment 22 Carlos Soriano 2016-04-14 08:36:41 UTC
Created attachment 325911 [details] [review]
files-view: hide properties menu item when action disabled

If not the desktop will show it with insensitive, which is odd.
Comment 23 Carlos Soriano 2016-04-14 08:36:47 UTC
Created attachment 325912 [details] [review]
files-view: use inheritance for querying special selection

There is a few times that the file cannot be copied, moved etc.
This usually happens with the special links in the desktop.
We were querying whether the selection has special items or not
in the files-view itself. However, this is something that every view
could have special, like the desktop one.

So use inheritance for it.
Comment 24 Carlos Soriano 2016-04-14 08:36:54 UTC
Created attachment 325913 [details] [review]
files-view: use inheritance for scripts vars

When a script is present we set some environment variables to allow
the scripts to work with them.

For example, we set a environment variable for selected items.
However, some views have special links, like the desktop. Therefore we
need special treatment for those.

Use inheritance instead of doing special casing in the parent class
for this case.
Comment 25 Carlos Soriano 2016-04-14 08:37:01 UTC
Created attachment 325914 [details] [review]
window-slot: make it derivable class
Comment 26 Carlos Soriano 2016-04-14 08:37:07 UTC
Created attachment 325915 [details] [review]
window: allow to create custom slots in subclasses

We are not able to create subclasses of window slots, like the one
we will need for desktop.

So create a vfunc to handle that, and expose the common code in a public
function so subclasses can use it.
Comment 27 Carlos Soriano 2016-04-14 08:37:13 UTC
Created attachment 325916 [details] [review]
desktop-window: create custom slot

We want to control few code paths, like the creation of the
desktop canvas, which currently is done by special casting in the window
slot instead of subclassing.

So create a window slot subclass that will serve for these purposes.
Comment 28 Carlos Soriano 2016-04-14 08:37:20 UTC
Created attachment 325917 [details] [review]
window-slot: allow overriding the creation of views

We would want in subclasses of nautilus window slot to have a way to
create custom views, like for instance, the desktop.

To allow it, make a vfunc for the creation of views that subclasses can
override.
Comment 29 Carlos Soriano 2016-04-14 08:37:26 UTC
Created attachment 325918 [details] [review]
desktop-window-slot: override creation of views

Now that we can create our own custom views in subclasses of the window
slot, we can remove the special casing for subclasses in window slot and
instead use inheritance for it.

This commit move the creation of the desktop view to the desktop slot.
Comment 30 Carlos Soriano 2016-04-14 08:37:32 UTC
Created attachment 325919 [details] [review]
window-slot: use action state instead of special casing the search

We were special casing to not enable search in some cases where some
special view, like the desktop, doesn't allow it.

But that shouldn't be special cased here, but instead use a subclass of
the window slot and disable it there.

Now that we have that, move the special casing to inheritance for
disabling the search in the desktop.
Comment 31 Carlos Soriano 2016-04-14 08:37:38 UTC
Created attachment 325920 [details] [review]
window-slot: remove unneeded code special casing desktop

We are special casing to open the desktop, so in case we try to
open the desktop in this window slot, and was already opened, we
were opening it again.

I cannot see a reason why we would need to open it anyway if was
already opened, and the test I did works fine.

So let's remove it for now, so we remove another special casing of the
desktop.
Comment 32 Carlos Soriano 2016-04-14 08:37:45 UTC
Created attachment 325921 [details] [review]
window-slot: use inheritance for other locations view

We need to special case the other locations view when using that
location, since it's not a files-view and doesn't support several things
that we usually support, like the changes between icon view and list
view.

Also we specifically special case its creation in window slot and we
disable few actions that are not available on it.

This patch creates a other locations slot, which will handle all of it.
The class that is responsible of creating one type of slot or another is
the window, and will use a vfunc that will request whether the slot
handles a location or not and will act accordingly.

In upcoming patches we will move all the special casing of this and the
desktop in the window slot to its respective subclasses now that we have
everything ready.
Comment 33 Carlos Soriano 2016-04-14 08:37:51 UTC
Created attachment 325922 [details] [review]
window-slot: use inheritance for disabling actions

Now that we have the necessary subclasses for special slots and views
like the desktop and the other locations one, we can rely on those for
disabling the actions that are not available on those and check for
those actions state instead of special casing subclasses.
Comment 34 Carlos Soriano 2016-04-14 08:37:58 UTC
Created attachment 325923 [details] [review]
window-slot: remove all dependencies to desktop

Now that we are free of special casing.
Comment 35 Carlos Soriano 2016-04-14 08:38:04 UTC
Created attachment 325924 [details] [review]
file, directory: move file creation dispatching to directory

It's the directory who owns the files, and we were actually creating
new files from nautilus directories in nautilus-file.

We can move that code to nautilus directory.

In this way, we will able to handle files of a specific scheme better,
like the desktop, in future patches.
Comment 36 Carlos Soriano 2016-04-14 08:38:11 UTC
Created attachment 325925 [details] [review]
directory: allow overriding the creation of files

So subclasses like the desktop directory can do their own.
Comment 37 Carlos Soriano 2016-04-14 08:38:17 UTC
Created attachment 325926 [details] [review]
desktop-directory: override creation of files

So we use inheritance instead of special casing, making possible
to remove the desktop dependency for the creation of the desktop
files.
Comment 38 Carlos Soriano 2016-04-14 08:38:24 UTC
Created attachment 325927 [details] [review]
desktop: ensure desktop directory on application init

Until now we were creating the desktop directory in a lazy way, like
any other cached directory.

However, we have the problem that at some point we have to dispatch
between different types of files, when creating them for the cache.

We cannot know when we will need to create the desktop directory for
first time in order to discern between that type of directory, or the
regular one.

What we can do is ensure that we created the desktop directory before
any other part of nautilus request it. In this way, we can create it
on our subclasses of the desktop, and after that, nautilus will request
the cache as a regular use, without the need to special case the
desktop.

For that, create the desktop directory when the desktop application
starts, holding the reference so the cache doesn't release it, and then
let nautilus work as expected.

For that, in previous commits we moved the file dispatching to be inside
the directory, so now any file creation happens inside the directory,
and therefore we can control, when creating the desktop directory, what
subclass will be called.
Comment 39 Carlos Soriano 2016-04-14 08:38:30 UTC
Created attachment 325928 [details] [review]
file: allow overriding rename handling

We will need it for using inheritance instead of special casing.
Comment 40 Carlos Soriano 2016-04-14 08:38:37 UTC
Created attachment 325929 [details] [review]
desktop-icon-file, file: move rename handling to subclass

Now that we can override the vfuncs needed for special casing the
desktop icon files, move all that handling to the subclass.
Comment 41 Carlos Soriano 2016-04-14 08:38:43 UTC
Created attachment 325930 [details] [review]
file: allow overriding of get_drop_target_uri

So subclasses like desktop icon can make its own handling, instead
of special casing in the parent.
Comment 42 Carlos Soriano 2016-04-14 08:38:50 UTC
Created attachment 325931 [details] [review]
file: rename get_drop_target_uri

This get target uri is used for those which we have special handling on
the activation uri, instead of the usual glib attribute
G_FILE_ATTRIBUTE_STANDARD_TARGET_URI. So it behaves like a supergroup
of those which have G_FILE_ATTRIBUTE_STANDARD_TARGET_URI, but we use
it in the same way.

We use for handling special files like desktop links on drag and drop,
etc.

However, this is useful for other parts of nautilus, and we will use it
in a upcoming patch, so we can rename it in order to communicate the
wide nature of the function.
Comment 43 Carlos Soriano 2016-04-14 08:38:56 UTC
Created attachment 325932 [details] [review]
desktop-icon-file: override get_target_uri

Now that we can do it in subclasses, use inheritance instead of special
casing in the parent.
Comment 44 Carlos Soriano 2016-04-14 08:39:03 UTC
Created attachment 325933 [details] [review]
file-dnd: move dnd handling to file

We are requiring file handling like special casing subclasses in the
dnd handling.

We are currently doing it outside of nautilus class... which makes
overriding on subclasses impossible.

This design goes against inheritance design, so there is no point on
making it that way if it defeats this purpose.

So merge the handling of file dnd inside the file class itself, and in
upcoming patches we will override that handling in order to use
inheritance instead of special casing in the parent.
Comment 45 Carlos Soriano 2016-04-14 08:39:10 UTC
Created attachment 325934 [details] [review]
file: allow overriding dnd handling

Subclasses may want to override some dnd handling.
Comment 46 Carlos Soriano 2016-04-14 08:39:16 UTC
Created attachment 325935 [details] [review]
desktop-icon-file: move dnd handling to the subclass

Now that we can use inheritance for dnd handling, use it in the subclass
instead of special casing on the parent.
Comment 47 Carlos Soriano 2016-04-14 08:39:23 UTC
Created attachment 325936 [details] [review]
file: allow overriding invalidate_attributes_internal

We are special casing some cases where we want to do something different
in subclasses.

Allow to override this function so subclasses can use it.
Comment 48 Carlos Soriano 2016-04-14 08:39:30 UTC
Created attachment 325937 [details] [review]
desktop-icon-file, file: override invalidate_attributes

Since the desktop files needs to do nothing at all.

This removes the last desktop dependency from nautilus file (yay).
Comment 49 Carlos Soriano 2016-04-14 08:39:37 UTC
Created attachment 325938 [details] [review]
file-operations: remove obsoleted desktop dependency
Comment 50 Carlos Soriano 2016-04-14 08:39:44 UTC
Created attachment 325939 [details] [review]
program-choosing: remove unneeded desktop dependency
Comment 51 Carlos Soriano 2016-04-14 08:39:50 UTC
Created attachment 325940 [details] [review]
window: use disable-chrome instead of desktop casting

We use disable-chrome all around to not show anything that comes
from the chrome of the window.

We can use it on all the cases, so we can avoid to special casing the
desktop.
Comment 52 Carlos Soriano 2016-04-14 08:39:57 UTC
Created attachment 325941 [details] [review]
mime-actions: move opens_in_view to file

And allow overriding of subclasses.

Nautilus mime actions file currently uses the file as parameter, as if
it was part of the nautilus file class.

However it's hosted in a different file, even though we need to special
case some subclasses of nautilus file for its management.
imho, this code design is wrong, since it defeats the purpose of OOP.

So move it to the nautilus file, allow overriding, and override this
vfunc in the subclasses that need it, like the desktop-icon.
Comment 53 Carlos Soriano 2016-04-14 08:40:04 UTC
Created attachment 325942 [details] [review]
toolbar: use widget visibility instead of desktop special casing

We need to avoid showing the operations popover in the desktop, or
basically in any window that has the disable-chrome property set.

We can check inside the toolbar if the toolbar itself is hidden in order
to know whether it makes sense to show the operations popover.

In this way we don't relay in special casing subclasses of the window.
Comment 54 Carlos Soriano 2016-04-14 08:40:11 UTC
Created attachment 325943 [details] [review]
canvas-view: remove unneeded desktop dependency
Comment 55 Carlos Soriano 2016-04-14 08:40:18 UTC
Created attachment 325944 [details] [review]
other-locations-window-slot: remove unneeded desktop dependency
Comment 56 Carlos Soriano 2016-04-14 08:40:25 UTC
Created attachment 325945 [details] [review]
properties-window: use get_target_uri instead of custom code

Now that we have the code to request the target uri in nautilus file,
we can use that instead of our own code special casing subclasses of
nautilus file.
Comment 57 Carlos Soriano 2016-04-14 08:40:31 UTC
Created attachment 325946 [details] [review]
properties-window: use app info for "Open With" visibility

We were special casing desktop icon files to not show the Open With
tab in the properties window.

However, this is just equal to what we need to do to display Open With
in the context menus, where we check if the app has a possible
application handling it or not.

Do the same here so we can remove another desktop dependency.
Comment 58 Carlos Soriano 2016-04-14 08:40:39 UTC
Created attachment 325947 [details] [review]
canvas-view: allow creation of canvas view container subclasses

This will be used by the desktop, so it can use its own
canvas-view-container.
Comment 59 Carlos Soriano 2016-04-14 08:40:46 UTC
Created attachment 325948 [details] [review]
canvas-view-container: provide autoptr cleanup

So subclasses can use G_DEFINE_FINAL_TYPE.
Comment 60 Carlos Soriano 2016-04-14 08:40:53 UTC
Created attachment 325949 [details] [review]
desktop-canvas-view: override canvas container creation

So we can use a subclass for the desktop.

This will be necessary in upcoming patches in order to handle the
special links on desktop that currently are done special casing inside
the parent canvas view container.
Comment 61 Carlos Soriano 2016-04-14 08:41:00 UTC
Created attachment 325950 [details] [review]
desktop-canvas-view-container: override get icon description

Now that we have a subclass we can override the function instead of
special casing in the parent.
Comment 62 Carlos Soriano 2016-04-14 08:41:07 UTC
Created attachment 325951 [details] [review]
desktop-canvas-view-container: override get icon text

Now that we have a subclass we can override the function instead of
special casing in the parent.
Comment 63 Carlos Soriano 2016-04-14 08:41:13 UTC
Created attachment 325952 [details] [review]
desktop-canvas-view-container: override compare icons

Now that we have a subclass we can override the function instead of
special casing in the parent.

Also we can remove the internal boolean "sort for desktop".

And finally we can remove any desktop dependency in canvas view container.
Comment 64 Carlos Soriano 2016-04-14 08:41:20 UTC
Created attachment 325953 [details] [review]
main-desktop: remove unneeded dependencies
Comment 65 Carlos Soriano 2016-04-14 08:41:28 UTC
Created attachment 325954 [details] [review]
desktop-application: chain up in dispose
Comment 66 Carlos Soriano 2016-04-14 08:41:35 UTC
Created attachment 325955 [details] [review]
desktop-application: remove unused priv data
Comment 67 Carlos Soriano 2016-04-14 08:41:42 UTC
Created attachment 325956 [details] [review]
desktop-application: use g_clear_error
Comment 68 Carlos Soriano 2016-04-14 08:41:49 UTC
Created attachment 325957 [details] [review]
desktop-application: create proxy synchronously

We are in the startup path, it's fine to do synchronously.
Comment 69 Carlos Soriano 2016-04-14 08:41:56 UTC
Created attachment 325958 [details] [review]
canvas-dnd: set as NULL after free

So we don't double free it.
Comment 70 Carlos Soriano 2016-04-14 08:42:04 UTC
Created attachment 325959 [details] [review]
desktop-canvas-view: remove unneeded check

There is no way the desktop view will hold a directory that is not
a desktop directory.
Comment 71 Carlos Soriano 2016-04-14 08:42:10 UTC
Created attachment 325960 [details] [review]
desktop-canvas-view: move comment inside the function
Comment 72 Carlos Soriano 2016-04-14 08:42:18 UTC
Created attachment 325961 [details] [review]
files-view: fix typo
Comment 73 Carlos Soriano 2016-04-14 08:42:24 UTC
Created attachment 325962 [details] [review]
files-view: move comment inside the function
Comment 74 Carlos Soriano 2016-04-14 08:42:32 UTC
Created attachment 325963 [details] [review]
file: move special link to file instead of the view

This feels like belonging to the file itself rather than the view.
Comment 75 Carlos Soriano 2016-04-14 08:42:39 UTC
Created attachment 325964 [details] [review]
files-view: use target_uri for script vars

The code was doing basically that in a hidden way. Let's simplify it.
Comment 76 Carlos Soriano 2016-04-14 08:42:47 UTC
Created attachment 325965 [details] [review]
window-slot: fix formatting
Comment 77 Carlos Soriano 2016-04-14 08:42:53 UTC
Created attachment 325966 [details] [review]
window: fix file leak
Comment 78 Carlos Soriano 2016-04-14 08:43:01 UTC
Created attachment 325967 [details] [review]
window: fix typo
Comment 79 Carlos Soriano 2016-04-14 08:43:07 UTC
Created attachment 325968 [details] [review]
window: fix formatting
Comment 80 Carlos Soriano 2016-04-14 08:43:15 UTC
Created attachment 325969 [details] [review]
window: use target_slot, not active slot

Since that's the one we are going to use.
Comment 81 Carlos Soriano 2016-04-14 08:43:22 UTC
Created attachment 325970 [details] [review]
window: use helper for replacing slot

Also, make the code cleaner.
Comment 82 Carlos Soriano 2016-04-14 08:43:29 UTC
Created attachment 325971 [details] [review]
file: extract shared code for handling gone files on renaming

Just to make it cleaner.
Comment 83 Carlos Soriano 2016-04-14 08:43:36 UTC
Created attachment 325972 [details] [review]
desktop-icon-file: chain up for get_target_uri

In case we don't meet the conditions the parent function can still
deal with it.
Comment 84 Carlos Soriano 2016-04-14 08:43:43 UTC
Created attachment 325973 [details] [review]
desktop-icon-file: fix missing parameter in vfunc
Comment 85 Carlos Soriano 2016-04-14 08:43:50 UTC
Created attachment 325974 [details] [review]
toolbar: use gtk_widget_is_visible for robustness

It checks the parents visibility as well.
Comment 86 Carlos Soriano 2016-04-14 08:43:58 UTC
Created attachment 325975 [details] [review]
desktop-canvas-view: fix wording
Comment 87 Carlos Soriano 2016-04-14 08:44:05 UTC
Created attachment 325976 [details] [review]
desktop-canvas-view-container: fix formatting
Comment 88 Carlos Soriano 2016-04-14 08:44:12 UTC
Created attachment 325977 [details] [review]
desktop-canvas-view-container: chain up to the parent

A mistake to chain up to the same class, creating a loop.
Comment 89 Carlos Soriano 2016-04-14 08:44:19 UTC
Created attachment 325978 [details] [review]
desktop-canvas-view-container: chain up to the parent

Same mistake as previous commit.
Comment 90 Carlos Soriano 2016-04-14 08:44:27 UTC
Created attachment 325979 [details] [review]
directory: use gio extensions for directory dispatching

We needed to do some hackish code in order to allow types that are
not included in nautilus-directory to dispatch the correct subclass.

Instead of that, we can just create a "plugabble" system that allows
directory types to be registered in the system, and implement a class
vfunc that queries if the class handles a specific type of uri, falling
back if none can handle it to the usual nautilus-directory.

We can do this for the desktop directory and the search directory.
Comment 91 Carlos Soriano 2016-04-14 08:44:35 UTC
Created attachment 325980 [details] [review]
file-dnd: remove the files

We moved the code to nautilus-file, but forgot to remove the files.
Remove them.
Comment 92 Carlos Soriano 2016-04-14 08:50:59 UTC
What is left is a compile flag and create a root folder for the desktop.
I'm working on the later. Not sure how valuable is the former tho.

Attachment 325899 [details] pushed as d92b660 - application: allow overriding open_location_full
Attachment 325900 [details] pushed as 23ae538 - application: add common startup code
Attachment 325901 [details] pushed as b80390d - desktop: move to a different binary
Attachment 325902 [details] pushed as 5e6026e - desktop-application: use dbus for opening files
Attachment 325903 [details] pushed as 149d383 - nautilus-dnd: request file system info if not available
Attachment 325904 [details] pushed as 8c94bc5 - files-view: remove unused merge/unmerge menus vfunc
Attachment 325905 [details] pushed as 043391c - files-view: remove unused can_rename_file vfunc
Attachment 325906 [details] pushed as f93021c - files-view: remove unused is_read_only vfunc
Attachment 325907 [details] pushed as fd0acab - files-view: remove unused get_icon_locations vfunc
Attachment 325908 [details] pushed as 71f7888 - files-view: use inheritance for showing the properties menu item
Attachment 325909 [details] pushed as 2ed64ca - files-view: use inheritance for get_backing_uri
Attachment 325910 [details] pushed as d713160 - files-view: use inheritance for empty states
Attachment 325911 [details] pushed as cff8dfc - files-view: hide properties menu item when action disabled
Attachment 325912 [details] pushed as 5ff1d1b - files-view: use inheritance for querying special selection
Attachment 325913 [details] pushed as 3c66ac7 - files-view: use inheritance for scripts vars
Attachment 325914 [details] pushed as 0a512d0 - window-slot: make it derivable class
Attachment 325915 [details] pushed as a5a0b74 - window: allow to create custom slots in subclasses
Attachment 325916 [details] pushed as db88189 - desktop-window: create custom slot
Attachment 325917 [details] pushed as 733b537 - window-slot: allow overriding the creation of views
Attachment 325918 [details] pushed as 5c414bf - desktop-window-slot: override creation of views
Attachment 325919 [details] pushed as 16b9778 - window-slot: use action state instead of special casing the search
Attachment 325920 [details] pushed as ef1d152 - window-slot: remove unneeded code special casing desktop
Attachment 325921 [details] pushed as 5f295bd - window-slot: use inheritance for other locations view
Attachment 325922 [details] pushed as 8468afe - window-slot: use inheritance for disabling actions
Attachment 325923 [details] pushed as a103c44 - window-slot: remove all dependencies to desktop
Attachment 325924 [details] pushed as f62fd13 - file, directory: move file creation dispatching to directory
Attachment 325925 [details] pushed as 1fb2ee1 - directory: allow overriding the creation of files
Attachment 325926 [details] pushed as f9d53f5 - desktop-directory: override creation of files
Attachment 325927 [details] pushed as 3d25404 - desktop: ensure desktop directory on application init
Attachment 325928 [details] pushed as 029da81 - file: allow overriding rename handling
Attachment 325929 [details] pushed as 796f320 - desktop-icon-file, file: move rename handling to subclass
Attachment 325930 [details] pushed as 196429f - file: allow overriding of get_drop_target_uri
Attachment 325931 [details] pushed as 8c2b6f2 - file: rename get_drop_target_uri
Attachment 325932 [details] pushed as 2136290 - desktop-icon-file: override get_target_uri
Attachment 325933 [details] pushed as 487cec0 - file-dnd: move dnd handling to file
Attachment 325934 [details] pushed as 1233066 - file: allow overriding dnd handling
Attachment 325935 [details] pushed as ab9d4a5 - desktop-icon-file: move dnd handling to the subclass
Attachment 325936 [details] pushed as e5cb376 - file: allow overriding invalidate_attributes_internal
Attachment 325937 [details] pushed as ea60660 - desktop-icon-file, file: override invalidate_attributes
Attachment 325938 [details] pushed as e6d9351 - file-operations: remove obsoleted desktop dependency
Attachment 325939 [details] pushed as 5e93be8 - program-choosing: remove unneeded desktop dependency
Attachment 325940 [details] pushed as b639a6d - window: use disable-chrome instead of desktop casting
Attachment 325941 [details] pushed as cc31a95 - mime-actions: move opens_in_view to file
Attachment 325942 [details] pushed as 1863c30 - toolbar: use widget visibility instead of desktop special casing
Attachment 325943 [details] pushed as ada4f7d - canvas-view: remove unneeded desktop dependency
Attachment 325944 [details] pushed as 4dc12ab - other-locations-window-slot: remove unneeded desktop dependency
Attachment 325945 [details] pushed as a5f7263 - properties-window: use get_target_uri instead of custom code
Attachment 325946 [details] pushed as 1caab8d - properties-window: use app info for "Open With" visibility
Attachment 325947 [details] pushed as f7abcb7 - canvas-view: allow creation of canvas view container subclasses
Attachment 325948 [details] pushed as e135555 - canvas-view-container: provide autoptr cleanup
Attachment 325949 [details] pushed as 43f6583 - desktop-canvas-view: override canvas container creation
Attachment 325950 [details] pushed as 9f85cff - desktop-canvas-view-container: override get icon description
Attachment 325951 [details] pushed as 9e2c045 - desktop-canvas-view-container: override get icon text
Attachment 325952 [details] pushed as 5f769aa - desktop-canvas-view-container: override compare icons
Attachment 325953 [details] pushed as e6b1d09 - main-desktop: remove unneeded dependencies
Attachment 325954 [details] pushed as 98c2570 - desktop-application: chain up in dispose
Attachment 325955 [details] pushed as 66ad6be - desktop-application: remove unused priv data
Attachment 325956 [details] pushed as 9997a74 - desktop-application: use g_clear_error
Attachment 325957 [details] pushed as aadc5fe - desktop-application: create proxy synchronously
Attachment 325958 [details] pushed as ac2b759 - canvas-dnd: set as NULL after free
Attachment 325959 [details] pushed as bdb248f - desktop-canvas-view: remove unneeded check
Attachment 325960 [details] pushed as ce000c8 - desktop-canvas-view: move comment inside the function
Attachment 325961 [details] pushed as 1e920dc - files-view: fix typo
Attachment 325962 [details] pushed as 391839b - files-view: move comment inside the function
Attachment 325963 [details] pushed as 191fbb4 - file: move special link to file instead of the view
Attachment 325964 [details] pushed as 32dbe75 - files-view: use target_uri for script vars
Attachment 325965 [details] pushed as f5002e8 - window-slot: fix formatting
Attachment 325966 [details] pushed as cc376b6 - window: fix file leak
Attachment 325967 [details] pushed as 96fb17a - window: fix typo
Attachment 325968 [details] pushed as 4c6d4e3 - window: fix formatting
Attachment 325969 [details] pushed as 9ed7dca - window: use target_slot, not active slot
Attachment 325970 [details] pushed as f8ad362 - window: use helper for replacing slot
Attachment 325971 [details] pushed as 6f364bd - file: extract shared code for handling gone files on renaming
Attachment 325972 [details] pushed as b5f3236 - desktop-icon-file: chain up for get_target_uri
Attachment 325973 [details] pushed as d8f481b - desktop-icon-file: fix missing parameter in vfunc
Attachment 325974 [details] pushed as 9091fe3 - toolbar: use gtk_widget_is_visible for robustness
Attachment 325975 [details] pushed as 9224ad5 - desktop-canvas-view: fix wording
Attachment 325976 [details] pushed as ea4be26 - desktop-canvas-view-container: fix formatting
Attachment 325977 [details] pushed as ae470f6 - desktop-canvas-view-container: chain up to the parent
Attachment 325978 [details] pushed as ae470f6 - desktop-canvas-view-container: chain up to the parent
Attachment 325979 [details] pushed as 7840b53 - directory: use gio extensions for directory dispatching
Attachment 325980 [details] pushed as edbd7b0 - file-dnd: remove the files
Comment 93 Carlos Soriano 2016-04-14 09:02:48 UTC
accidentally marked as fixed.
Comment 94 Carlos Soriano 2016-04-15 18:12:10 UTC
Created attachment 326125 [details] [review]
desktop: move to a different folder

For a better structured hierarchy.
Comment 95 Carlos Soriano 2016-04-15 18:13:34 UTC
Comment on attachment 326125 [details] [review]
desktop: move to a different folder

Only thing left now is a compile switch.

Attachment 326125 [details] pushed as 73e10fa - desktop: move to a different folder
Comment 96 Carlos Soriano 2016-04-19 14:51:22 UTC
Created attachment 326328 [details] [review]
desktop: add a compile flag for building it

So we can build nautilus without desktop support and the other way
around.
Comment 97 Carlos Soriano 2016-04-19 14:52:26 UTC
Now yes, a compile switch is added. This bug is fixed.

Attachment 326328 [details] pushed as 9bb4ff0 - desktop: add a compile flag for building it