Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bulk renamer plugin #1974

Closed
wants to merge 22 commits into from
Closed

Bulk renamer plugin #1974

wants to merge 22 commits into from

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Jan 7, 2022

Fixes #1040

Functional but needs UX of dialog sorting out. Suggestions welcome.

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this is a lot! First thing, instead of a separate plugin/menuitem, this should probably just be the behavior for the Rename menuitem when multiple files are selected

I kind of feel like this is probably overwhelming for most people to the point of not longer being useful. Perhaps we should scope down here to just simple renaming schemes and leave some of the more advanced stuff for specialized apps if you really have a wild use case? I'd guess the vast majority of bulk renaming operations would be to suffix with either numbers or a date, but probably not multiple combinations of prefixes, suffixes, and dates.

The GNOME approach here is also interesting. It uses a single entry where you can add items from a menu to make a regex-like template:

nautilus-batch-rename

This seems like it would allow for really flexible renaming while keeping things simple for someone to specify a format like [Original file name]-[1, 2, 3].

*
*/

public class Files.RenamerDialog : Gtk.Dialog {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Granite.Dialog here would save some styling work

}

construct {
deletable = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dialogs shouldn't have close window controls since it's ambiguous what happens when the window is closed.

Suggested change
deletable = true;

Comment on lines 100 to 102
realize.connect (() => {
resize (500, 300); //Stops the window being larger than necessary
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should find the actual cause of this, this seems pretty janky

Comment on lines 82 to 90
switch (keyval) {
case Gdk.Key.Escape:
if (mods == 0) {
response (Gtk.ResponseType.REJECT);
}
break;
case Gdk.Key.Return:
if (mods == 0 && renamer.can_rename) {
response (Gtk.ResponseType.APPLY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely isn't the right way to do this. the "Rename" button should have has_default = true and entry widgets in the dialog should have activates_default = true

modifier_chain = new Gee.ArrayList<Modifier> ();

var base_name_label = new Granite.HeaderLabel (_("Base"));
base_name_label.get_style_context ().add_class (Granite.STYLE_CLASS_H2_LABEL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making these header labels this big seems pretty excessive

base_name_combo.insert (RenameBase.CUSTOM, "CUSTOM", RenameBase.CUSTOM.to_string ());

base_name_entry = new Gtk.Entry () {
placeholder_text = _("Enter naming scheme"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placeholder text should be an example of valid text for this entry. Descriptions of entries should be labels outside the entry. I'm not sure what good example text here would be, so we should probably remove this

base_name_entry = new Gtk.Entry () {
placeholder_text = _("Enter naming scheme"),
hexpand = false,
max_width_chars = 64,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for restricting the character limit here?


base_name_entry = new Gtk.Entry () {
placeholder_text = _("Enter naming scheme"),
hexpand = false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expand is false by default, so we don't need to specify it

Suggested change
hexpand = false,

Comment on lines 100 to 123
var sort_by_grid = new Gtk.Grid () {
orientation = Gtk.Orientation.HORIZONTAL,
column_spacing = 6,
halign = Gtk.Align.END,
valign = Gtk.Align.CENTER
};
sort_by_grid.add (sort_by_label);
sort_by_grid.add (sort_by_combo);

var sort_type_label = new Gtk.Label (_("Reverse"));

sort_type_switch = new Gtk.Switch () {
valign = Gtk.Align.CENTER
};

var sort_type_grid = new Gtk.Grid () {
orientation = Gtk.Orientation.HORIZONTAL,
column_spacing = 6,
halign = Gtk.Align.END,
valign = Gtk.Align.CENTER,
margin = 3
};
sort_type_grid.add (sort_type_switch);
sort_type_grid.add (sort_type_label);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems kind of excessive. Do we really need sort controls here?

@jeremypw
Copy link
Collaborator Author

Thanks for the review. This was originally written as a separate app but I never got round to submitting it to AppCenter as I had difficulty coming up with a suitable icon/name. I submitted it here in response to renewed interest in the related issue.

I'll have a think about simplifying the UX so only the most commonly used options appear initially and perhaps hide less used options under an "Advanced..." button or something. The screenshot does show more modifiers than most people would use just to illustrate that you can chain an arbitrary number of them. The dialog does need to be made smaller.

Regarding having only one option "Rename", I have also submitted an alternative way the existing option could handle a multiple selection, i.e. by sequentially renaming them arbitrarily (which this PR cannot do at present). I guess in most cases where a user wants to rename multiple files it is in order to apply some pattern though.

@jeremypw
Copy link
Collaborator Author

The code is quite old so definitely needs updating to best practices!

@jeremypw
Copy link
Collaborator Author

I can drop the sorting of the original files and have it follow the current sort order of the view.

@jeremypw jeremypw marked this pull request as draft January 17, 2022 13:03
@jeremypw
Copy link
Collaborator Author

Closed in favour of #1999, which is not a plugin and has a simpler interface.

@jeremypw jeremypw closed this Jan 28, 2022
@jeremypw jeremypw deleted the bulk-rename-plugin branch January 28, 2022 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bulk rename
2 participants