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 770199 - Add support for compressing files
Add support for compressing files
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-21 13:49 UTC by Razvan Chitu
Modified: 2016-08-22 21:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
file-operations: implement compression operation (25.42 KB, patch)
2016-08-21 13:49 UTC, Razvan Chitu
none Details | Review
file-operations: implement compression operation (25.62 KB, patch)
2016-08-21 16:41 UTC, Razvan Chitu
committed Details | Review
general: add a setting for the default compression format (3.99 KB, patch)
2016-08-21 20:06 UTC, Razvan Chitu
none Details | Review
files-view: add menu action and dialog for compression (23.60 KB, patch)
2016-08-21 20:06 UTC, Razvan Chitu
none Details | Review
general: add a setting for the default compression format (4.13 KB, patch)
2016-08-21 22:29 UTC, Razvan Chitu
committed Details | Review
files-view: add menu action and dialog for compression (32.01 KB, patch)
2016-08-22 01:06 UTC, Razvan Chitu
none Details | Review
files-view: add menu action and dialog for compression (32.01 KB, patch)
2016-08-22 01:17 UTC, Razvan Chitu
none Details | Review
files-view: add menu action and dialog for compression (31.83 KB, patch)
2016-08-22 09:26 UTC, Razvan Chitu
none Details | Review
files-view: add menu action and dialog for compression (31.90 KB, patch)
2016-08-22 12:21 UTC, Razvan Chitu
committed Details | Review
file-name-widgets: use a revealer to display errors (11.96 KB, patch)
2016-08-22 12:22 UTC, Razvan Chitu
none Details | Review
file-name-widgets: use a revealer to display errors (12.31 KB, patch)
2016-08-22 17:05 UTC, Razvan Chitu
committed Details | Review

Description Razvan Chitu 2016-08-21 13:49:32 UTC
See patch.
Comment 1 Razvan Chitu 2016-08-21 13:49:36 UTC
Created attachment 333819 [details] [review]
file-operations: implement compression operation

Add an operation for compressing files using gnome-autoar.
Comment 2 Carlos Soriano 2016-08-21 14:22:23 UTC
Review of attachment 333819 [details] [review]:

About the commit message, would be good to explain the rationale as you explained well for the decompression. We know we wanted this and the reasons we want this, but not the developer coming in 2 years :)

You got the code right though, nice!

::: src/nautilus-file-undo-operations.h
@@ +350,3 @@
+GType nautilus_file_undo_info_compress_get_type (void) G_GNUC_CONST;
+NautilusFileUndoInfo * nautilus_file_undo_info_compress_new (GList        *sources,
+

identation
Comment 3 Razvan Chitu 2016-08-21 16:41:54 UTC
Created attachment 333827 [details] [review]
file-operations: implement compression operation

Add an operation for compressing files using gnome-autoar. The operation is
similar in functionality to the one offered by file roller but comes with
integrated progress feedback and support for undoing and redoing.
Comment 4 Razvan Chitu 2016-08-21 20:06:28 UTC
Created attachment 333839 [details] [review]
general: add a setting for the default compression format
Comment 5 Razvan Chitu 2016-08-21 20:06:33 UTC
Created attachment 333840 [details] [review]
files-view: add menu action and dialog for compression

Add an context menu action for compression and a dialog for selecting the file
name and compression format. Add a controller class for managing the compression
dialog.
Comment 6 Carlos Soriano 2016-08-21 22:12:16 UTC
Review of attachment 333827 [details] [review]:

+1
Comment 7 Carlos Soriano 2016-08-21 22:14:43 UTC
Review of attachment 333839 [details] [review]:

Add a description in the commit message

::: data/org.gnome.nautilus.gschema.xml
@@ +58,3 @@
 
+  <enum id="org.gnome.nautilus.CompressionFormat">
+    <value value="0" nick=".zip"/>

remove the first dot "."

@@ +223,3 @@
+      <default>'.zip'</default>
+      <summary>Default format for compressing files</summary>
+      <description>The format that will be automatically selected when compressing files.</description>

will be selected when...
Comment 8 Razvan Chitu 2016-08-21 22:29:51 UTC
Created attachment 333852 [details] [review]
general: add a setting for the default compression format

The compression operation allows multiple formats to be selected. It would be
good to store the last choice of the user in order to select it for future
operations.
Comment 9 Carlos Soriano 2016-08-21 22:43:17 UTC
Review of attachment 333840 [details] [review]:

::: src/nautilus-compress-dialog-controller.c
@@ +8,3 @@
+#include "nautilus-global-preferences.h"
+
+#define CUSTOM_EXTENSION_MAXIMUM_SIZE 4

not sure why we would need this... I think we agreed on using the last part of the file name to test this.

@@ +32,3 @@
+        { NAUTILUS_COMPRESSION_TAR_XZ, ".tar.xz", "Linux and Mac only but more efficient" },
+        { NAUTILUS_COMPRESSION_7ZIP, ".7z", "Efficient but must be installed on Windows and Mac" }
+};

OMG split it.
Also I assume the order has to be the same as the preferences enum, which is not clear.
Put a comment, since we do something similar in canvas view, although I don't like it much.

@@ +97,3 @@
+
+static void
+compress_dialog_controller_on_toggled (GtkToggleButton *toggle_button,

format_toggle_button_on_toggled, not need for the class prefix, but rather for the object type from the signal

@@ +119,3 @@
+                            _(format_details[format].description));
+
+        g_signal_emit_by_name (controller->name_entry, "changed");

you need to manually do this?

@@ +166,3 @@
+                                      NAUTILUS_PREFERENCES_DEFAULT_COMPRESSION_FORMAT);
+
+        for (i = 0; i < G_N_ELEMENTS (format_details); ++i) {

I think this is too complex for what we need... just stuff this all into the .ui file, then just connect specifically to the radio buttons like zip_toggle_button_on_toggled etc. Save complexity for something generic when it's just 3 items :)
In every callback you will set the appropriate description label, not need for that format_details struct from before.

@@ +220,3 @@
+nautilus_compress_dialog_controller_init (NautilusCompressDialogController *self)
+{
+        self->radio_to_format = g_hash_table_new (NULL, NULL);

not needed...make it simpler.

::: src/nautilus-files-view.c
@@ +1927,3 @@
+
+        if (!success) {
+        NautilusFile *file;

we use "out" instead of "fail"

::: src/resources/ui/nautilus-files-view-context-menus.ui
@@ +247,3 @@
       </item>
+      <item>
+        <attribute name="label" translatable="yes">Compress</attribute>

add a nmemonic, be careful to not clash with others
Comment 10 Carlos Soriano 2016-08-21 22:44:06 UTC
Review of attachment 333852 [details] [review]:

ok
Comment 11 Razvan Chitu 2016-08-22 01:06:53 UTC
Created attachment 333854 [details] [review]
files-view: add menu action and dialog for compression

Add an context menu action for compression and a dialog for selecting the file
name and compression format. Add a controller class for managing the compression
dialog.
Comment 12 Razvan Chitu 2016-08-22 01:17:08 UTC
Created attachment 333856 [details] [review]
files-view: add menu action and dialog for compression

Add an context menu action for compression and a dialog for selecting the file
name and compression format. Add a controller class for managing the compression
dialog.
Comment 13 Carlos Soriano 2016-08-22 08:45:57 UTC
Review of attachment 333856 [details] [review]:

::: src/nautilus-compress-dialog-controller.c
@@ +221,3 @@
+                                      NAUTILUS_PREFERENCES_DEFAULT_COMPRESSION_FORMAT);
+
+        switch (format) {

This looks like can be done much simpler calling update_on_toggle, or making a helper function that both of them call. But I think what you currently have is enough, just call it here.

@@ +243,3 @@
+
+        gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (self->active_button),
+                                      TRUE);

I think we can avoid this setting this in update_on_toggle. Toggling the already toggled button should do nothing, which is fine. In this way we have a way on the code to update the format selected.
Comment 14 Razvan Chitu 2016-08-22 09:26:07 UTC
Created attachment 333872 [details] [review]
files-view: add menu action and dialog for compression

Add an context menu action for compression and a dialog for selecting the file
name and compression format. Add a controller class for managing the compression
dialog.
Comment 15 Carlos Soriano 2016-08-22 09:28:19 UTC
Review of attachment 333872 [details] [review]:

now we are talking, much clear!
Comment 16 Razvan Chitu 2016-08-22 12:21:36 UTC
Created attachment 333898 [details] [review]
files-view: add menu action and dialog for compression

Add an context menu action for compression and a dialog for selecting the file
name and compression format. Add a controller class for managing the compression
dialog.
Comment 17 Razvan Chitu 2016-08-22 12:22:18 UTC
Created attachment 333899 [details] [review]
file-name-widgets: use a revealer to display errors

Each file name widget has an error label for displaying error messages. However,
when there is no error, the label just takes up space for no reason. In order to
fix this, use a revealer to display error messages.
Comment 18 Carlos Soriano 2016-08-22 15:51:06 UTC
Review of attachment 333899 [details] [review]:

yep!
Comment 19 Carlos Soriano 2016-08-22 16:50:16 UTC
Review of attachment 333898 [details] [review]:

The only change were the elipsis, so patch is fine :)
Comment 20 Razvan Chitu 2016-08-22 17:05:32 UTC
Created attachment 333931 [details] [review]
file-name-widgets: use a revealer to display errors

Each file name widget has an error label for displaying error messages. However,
when there is no error, the label just takes up space for no reason. In order to
fix this, use a revealer to display error messages.
Comment 21 Carlos Soriano 2016-08-22 17:35:29 UTC
Review of attachment 333931 [details] [review]:

ok
Comment 22 Razvan Chitu 2016-08-22 21:43:09 UTC
Attachment 333827 [details] pushed as 918f2b6 - file-operations: implement compression operation
Attachment 333852 [details] pushed as 9d8de33 - general: add a setting for the default compression format
Attachment 333898 [details] pushed as e9efd04 - files-view: add menu action and dialog for compression
Attachment 333931 [details] pushed as d0934db - file-name-widgets: use a revealer to display errors