GNOME Bugzilla – Bug 171416
Resume editing if name of new folder is "Type name of new folder"
Last modified: 2009-06-12 18:09:52 UTC
Currently it's very easy to accidentally create folders called "Type name of new folder", or its local translation. We should set a flag that says whether the used has changed the default string, and just resume editing if we are in the confirmation stage and the flag hasn't changed. [Also, resume editing if the string is empty; currently the file chooser says that you can't re-create the current folder because it exists: it's clearly appending "" to "/current/folder/" and getting back the "same" string :) ]
Mozilla has an interesting workaround for this problem. Instead of using the Windows-style approach, it pops up a dialog window asking for name for the new directory. If user hits OK, the directory will be created, but if he hits Cancel, the directory is not created. Very simple, very intuitive.
I think a simple fix would be to just allow renaming in the file list, by adding a "Rename" entry to the context menu.
Created attachment 50354 [details] [review] refuse with the default folder name and empty name As a gnome-lover, i try to work on this bug. This first patch seems to work well when you confirme the folder name by hitting enter key but doesn't when you click outside the cell during the editing: Gtk-CRITICAL **: _gtk_tree_view_column_start_editing: assertion `tree_column->editable_widget == NULL' failed and display problem into the tree view. I don't understand why it's not working in this cass. :\
Welcome aboard, Guillaume. We just need you. Several comments about your patch 1. Although it's very minor just to keep style it's better to use N_() instead of _() in defines. Look for example in gtkfilechooserbutton.c 2. It's better first check string for emptyness, only then compare it. This is more natural order of things. 3. I don't know why do you need to set cursor. Isn't it better just ignore call of queue_edited_idle. But you should call g_object_set (cell_renderer_text, "mode", GTK_CELL_RENDERER_MODE_INERT, NULL); in both cases. Probably it's the main reason of your pbm.
And, sorry, probably you should also use queue_edited_idle (impl, NULL); as in cancel function below.
*** Bug 314220 has been marked as a duplicate of this bug. ***
Created attachment 51356 [details] [review] Fixes refusing to create directory if name unchanged or empty.
*** Bug 346254 has been marked as a duplicate of this bug. ***
Could some body review this patch? Looks like the author fixed what Nickolay asked him to more than two years ago!
Well, I'm going to propose another patch since I find the previous to ones too complex for what they want to achieve. There's no need to check if the folder name was edited, we only have to see if it's empty or equal to the default name. If it's the case, just stop editing as if it was correct, but don't create the folder. This is the best behavior IMHO since there's no way to remove a bad folder in the file chooser. So if you change your mind and don't want to create that folder, you need a way to go out of this process, and avoid creating a nice folder called "Type name of new folder". If you mistakingly clicked out of the label, you can still click on "Create folder" again. The patch is only 6 lines and does not need to modify anything except adding two conditions to an already present if(). It works nicely so I think that solution is legitimate. Please comment, I'll change it needed! We can solve it, that's been opened for a long time...
Created attachment 129643 [details] [review] Patch against gtkfilechooserdefault.c in trunk
*** Bug 352916 has been marked as a duplicate of this bug. ***
Hmmm. For maintainability, Milan's patch is certainly better, and likely just as effective. Sorry to make you work more than needed, Claudio :) Did you mean N_("Type name...") or just _("Type name...")? I think you want _(), or the string won't be translated at all. If you make that change, please go ahead and commit. Thanks for the patch!
*** Bug 412083 has been marked as a duplicate of this bug. ***
I meant to use gettext_noop(), but I guess I was misled by a previous comment, as there's no need for it here. Just use _(). Funny, that's the second time I'm asked to commit a patch. I'd be glad to do so, but I've no commit rights, so you'll need to do this yourself... ;-) Thanks for having a look at all those patches, we really need to clean them!
Created attachment 129997 [details] [review] Fixed patch
Federico wrote: > If you make that change, please go ahead and commit. As I said some time ago, I don't have the required commit rights, so I need a dev to do this for me. Thanks!
Committed with minor whitespace changes as d87dbd66d6f4533df85e0967f9dfb45da0681c1e.