User-Agent: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.1.3) Gecko/20070322 Firefox/2.0.0.3 Build Identifier: After adding a command which has spaces in the filename, the command isn't quoted (put in quotes ""). Reproducible: Always Steps to Reproduce: 1. Create some dummy command and save it as "~/some dummy command" 2. Start thunar 3. Got to "User Defined Action" ("Benutzerdefinierte Aktion" in German. It's on the "Bearbeiten" ("Edit") menu. The 2nd to last option, right before "Einstellungen" (Settings) 4. Create a new action by clickng on the + (or edit an existing one) 5. Browse for the command and select "some dummy command" in $HOME; suppose $HOME=/home/alexander for this example Actual Results: As command, the following is entered: /home/alexander/some dummy command %f When I select this command by right clicking in a folder and selecting this newly created command, nothing happens. Expected Results: Clicking on the command should run the command. If the command would've been put in quotes (ie. "/home/alexander/some dummy command" instead of /home/alexander/some dummy command), clicking on the command would be possible. As it is right now, the user has to add these quotes by himself. This should not be the case.
You're kinda confusing things here... If you use commands with spaces in it (which is generally a pretty bad idea and doesn't make sense anyway), you must add quotes. For example /home/alexander/some dummy command %f How should software know what to quote? Only the first two strings, or the first three, or the whole thing?
(In reply to comment #1) > You're kinda confusing things here... I don't think so. > If you use commands with spaces in it > (which is generally a pretty bad idea and doesn't make sense anyway), Well, I disagree. It's a very good idea and naming commands in a senseful way does make sense. On the contrary: Naming commands in a way which does not make immediately sense, is genereally a pretty bad idea. It really does not make sense to refrain from using perfectly legal characters. BTW: You'd get the same broken behaviour of the UCA dialog, if the command is stored in a directory which has spaces in the name. > you must > add quotes. Yep. The software should be smart enough to add quotes, when there's a need. The need can be discovered quite easily: - Have user *BROWSE* for a command. - Does path to command, which user selected in the BROWSE DIALOG (that's important!) contain spaces in the name? - If yes, add quotes. > For example > > /home/alexander/some dummy command %f > > How should software know what to quote? Quite easy - user selected "some dummy command" in the browse dialog. "some dummy command" contains spaces, thus just "some dummy command" or "/home/alexander/some dummy command" is to be put in quotes (for bash, both ways would work). > Only the first two strings, or the > first three, or the whole thing? The first three strings (up to, and including, the word command), as that has been selected in the browse dialog and requires quotes to work. Anything else doesn't make sense, would it?
BTW: Could you please be so kind and point me to the source file of the UCA dialog? If so, I'd like to write a patch and submit it for (possible) inclusion.
Created attachment 1087 Patch against thunar-uca-editor.c, adding quotes The attached patch makes it so, that the filename has quotes (") before and after it, if quotes are required. Right now, it only checks for the occurance of space.
FWIW: I also added this to the bug tracking system of my distributor, Gentoo. See http://bugs.gentoo.org/show_bug.cgi?id=173795, if you're interested.
Any decision how this will be handled?
I think this bug should be marked as INVALID since quoting is completely wrong here.
Pardon me, but why is it "wrong" to quote a command, so that the generated command line is directly usable? As it is right now, if a user selects a command with spaces in the name (eg. "funny name"), a not working command line is generated. That's IMO broken. With the patch applied, the path is quoted. Eg. if the path to the command would be /home/user/bin/some funny command the following would be entered in the command line: "/home/user/bin/some funny command" %f why is it wrong to quote »/home/user/bin/some funny command«? Without quotes, the command cannot be run. Quotes are needed.
Mm now I see you only quote when using the search command button, that's something completely different as the title would suggest. Anyway, then some code like this is better: /* check if filename contains spaces, if so, add some quotes */ if (G_LIKELY (strstr (filename, ' ') == NULL)) s = g_strconcat (filename, " %f", NULL); else s = g_strconcat ("'", filename, "' %f", NULL);
(In reply to comment #9) > Mm now I see you only quote when using the search command button, Yes, exactly. That's why I kept on repeating, that the user should browse for a command ;) > that's > something completely different as the title would suggest. Okay. Sorry for not writing the summary as clear as it should've been. English is not my native language :-) > Anyway, then some > code like this is better: > > /* check if filename contains spaces, if so, add some quotes */ > if (G_LIKELY (strstr (filename, ' ') == NULL)) > s = g_strconcat (filename, " %f", NULL); > else > s = g_strconcat ("'", filename, "' %f", NULL); > In that case, you'd do away with the "g_strconcat (safe_filename, " %f", NULL);" after the "if" statement, wouldn't you? Well, if that code of yours is better, than that's cool with me. All I care about, is that this feature or bugfix is added to the code, so that users don't have to manually quote a command anymore, in case they select a command in the file browser which has spaces in the name.
Created attachment 1104 Escape command (after browsing) if it contains spaces Ok here is my patch (using strchr and avoids an unnecessary string duplication). It's up to Benny if he applies the patch or not. Sorry Alexander for not reading your patch properly.
Fixed with revision 25732. 2007-05-20 Benedikt Meurer <benny@xfce.org> * plugins/thunar-uca/thunar-uca-editor.c: Properly quote files selected via the file chooser if necessary. Bug #3105.