Created attachment 9586 patch to add new widget I propose that we add a new widget to libxfce4ui which provides a version of the GtkEntry widget especially for entering filenames for renaming or creating files, with as-you-type checking for errors such as a filename being too long or containing directory separator characters. The attached patch implements this feature, and a demonstrator program for this new widget is attached to the next comment. The widget displays an error if the entered filename is longer than the given maximum value or contains the directory separator. It warns the user if the given filename starts or ends with whitespace. The reason for adding this feature comes from trying to fix bug #13720 in Thunar. While investigating this, I realised that there are several places in the Thunar code where the user is asked for a filename for a new file or a new name for an existing file, and these do not behave the same. Thus the best option seemed to be a new widget featuring as-you-type checking of the filename, and alexxcons suggested that this new widget should be in libxfce4ui to allow reuse. If this widget is added to libxfce4ui, then I will patch Thunar to use it. Now this is the first time I have tried to implement a new GObject class, and I have done my best to get all of the magical GObject hocus-pocus right - I have had a careful look at existing code in libxfce4ui and elsewhere in Xfce, and done lots of googling and reading of documents. The patch compiles and runs without errors or warnings on my system. However, it is still likely that I have messed things up or done something dumb. If anyone who knows about this stuff has time to have a quick look at my code, that would be great! Comments and criticisms are most welcome!
Created attachment 9587 demonstration program for above patch Here is a demonstrator for the new widget. On my system it compiles with gcc demonstrator.c `pkg-config --cflags --libs libxfce4ui-2`
Created attachment 9588 formatting patch Nice, thanks alot for the patch ! Here some remarks: - I applied some formatting ... see first patch attached - There are two build warnings to be fixed "comparison of integer expressions of different signedness" - All methods which are public API should get documentation in the *.c file -- Use the following in the comments: "* Return value: (transfer none): blabla" in order to get rid of the build warning "xfce_filename_input_get_entry: return value: Missing (transfer) annotation" -- (https://gi.readthedocs.io/en/latest/annotations/giannotations.html#memory-and-lifecycle-management) - There is a "tests" folder in the project .. I did not try, though I guess you can put the code of the demonstrator there - Attached another patch which keeps the public API a bit smaller by using properties .. like that you can create the filename_input with: g_object_new (XFCE_TYPE_FILENAME_INPUT, "original-file-name", orig_name, "max-length", max_len, NULL); .... for some reason I dont understand, yet, I cannot test my changes with your demonstrator .. please feel free to test my patches (I only know that it compiles) and merge them into your patch, if you think it is apropriate.
Created attachment 9589 Adding properties for max-length and original-filename ... forgot to mention: - I used G_PARAM_CONSTRUCT_ONLY, like that you dont need to check for overwrite of the original-filename - Would be good to have "g_return_if_fail" and "g_return_val_if_fail" on each method to check XFCE_IS_FILENAME_INPUT()
Thanks Alex! I'll make the changes you suggest and get a revised patch up. One question: I initially used properties for max-length and original-filename, but I changed to not using them after reading somewhere that it's recommended only to use properties when users might want to listen for changes to them, as otherwise you get overhead from all the "changed" signals that they generate. Is that right? I cannot remember where I read it, but I think it was in an answer on some forum or other so possibly not very reliable. Thanks!
(In reply to Reuben Green from comment #4) > One question: I initially used properties for max-length and > original-filename, but I changed to not using them after reading somewhere > that it's recommended only to use properties when users might want to listen > for changes to them, as otherwise you get overhead from all the "changed" > signals that they generate. Is that right? I cannot remember where I read > it, but I think it was in an answer on some forum or other so possibly not > very reliable. Afaik you actively need to signal "change" for property-subscribers on property change. If you dont, nothing will be sent .. though I could be wrong. Anyway, our properties are only changed once, at creation, so I dont think it should matter here.
Created attachment 9612 patch to add new widget Here is a revised patch incorporating all of alexxcons's suggestions. Hopefully I've managed to get the formatting right this time! In particular I have added the new widget to the test program, so you can test it using that.
Thanks alot, looks very nice now ! > In particular I have added the new widget to the test program, so you can test it using that. That worked out nicely for me, thanks ! :) If you dont plan to do further modifications on it, I would push the new widget.
No, I don't have any further plans, unless anyone else has any checks they want added. Thanks!
Reuben Green referenced this bugreport in commit 8f0ad0c23947545abf249aac56124cde74cdeb0a Add a widget for filename input (Bug #16542) https://git.xfce.org/xfce/libxfce4ui/commit?id=8f0ad0c23947545abf249aac56124cde74cdeb0a
Ok, pushed to master. I justsaw that it might help to have "Since: 4.16" on the doc of all methods and added it. Thanks alot for your work !
Thanks Alex! I'll start preparing patches for Thunar to use this.