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

Add native support for using a bulk renaming tool. #1535

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mtwebster
Copy link
Contributor

This allows the normal method of renaming a file launch a bulk
renamer if one is configured and multiple files are selected.

Ported from nemo.

@mbkma
Copy link
Member

mbkma commented Jun 26, 2021

Thanks! It is very handy to have bulk rename built in. However, a few comments:

  • I do not think it is a good idea that the user has to enter a command manually in caja-preferences. The bulk rename tool should be automatically detected by the code, or hardcoded (such as in mate-panel window list applet).
  • It is currently possible to enter any command, so e.g. atril will open two instances of atril instead of renaming. This should be avoided.

I tested it with the thunar bulk renamer ( thunar --bulk-rename ) it works fine.

@mbkma mbkma requested a review from a team June 26, 2021 07:43
@mtwebster
Copy link
Contributor Author

Hi, thanks for the feedback -

1 - I'm not sure why we shouldn't allow the user to define the bulk renamer - this allows them to use whichever one they prefer - there are a number of them out there. A default can always be defined in the settings gschema file, as well as overridden by distributions. At the very least the setting should exist, even if you'd prefer not to have it in the preferences ui.

2 - We can't detect what is a bulk renamer and what isn't. Of course the user can put anything in there, but I think you have to give them some benefit of the doubt and assume they won't enter something in there that isn't actually a bulk renamer.

Thank you

@mbkma
Copy link
Member

mbkma commented Jul 8, 2021

Yep, I think just a gsettings key with a default is perfect. No need to expose this to the user in the preferences ui.

For convenience you could try to find some hard coded bulk-rename-tools. Maybe sth like this works:

static const char* bulk_rename_tools[] = {
	"bulky",
        ...
};

int i;
gchar *progname;
for (i = 0; i < G_N_ELEMENTS(bulk_rename_tools); i++)
{
    if ((progname = g_find_program_in_path(bulk_rename_tools[i])) != NULL);
    {
         g_free(progname);
         break;
    }
}

if (i < G_N_ELEMENTS(bulk_rename_tools))
    g_settings_set(settings, "bulk-rename-tool", "^ay", bulk_rename_tools[i]);

@mtwebster
Copy link
Contributor Author

I removed the preferences ui portion of this, and added some hardcoded renamers to check in addition to the gsettings key. If there are any popular ones I missed let me know and I'll add them.

@mbkma
Copy link
Member

mbkma commented Sep 17, 2021

Thanks a lot! In a quick test everything works fine.

g_free (quoted_parameter);
}

guint i = 0;
Copy link
Member

Choose a reason for hiding this comment

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

this i can be in the for-loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cleaned up the legacy code a bit (I should in nemo also), though I'm not really keen on declaring i in the loop init - it's not consistent with the rest of caja (and personally I think it worsens readability, but I'm old and blind).

This allows the normal method of renaming a file launch a bulk
renamer if one is detected (or configured) and multiple files are
selected.

Ported from nemo.
@lukefromdc
Copy link
Member

What is the status of this given the issues Mint had with Bulky?

@mtwebster
Copy link
Contributor Author

That issue isn't relevant - it was a problem with the pinned caja version in Mint, and its hardcoded use of bulky. I reviewed this PR since then to be sure, but it already guards against that problem.

@lukefromdc
Copy link
Member

What is the status of this? Are people still interested in working on it, should we close it, or something else?

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.

3 participants