Comma is not allowed in the file name

[Mingun]

Brief summary of issue

The , (comma) symbol treated as a special and cannot be entered to the name field, but it never had a special meaning in any OS as I known

Steps to reproduce the issue

1. Make screenshot
2. Press F2 to start a rename
3. Try to input , (comma)

Error output

Extra information, such as Shutter version, display server in use (Xorg or Wayland), operating system and ideas for how to solve:

Shutter version: 0.99.2 Rev.1593 OS: Ubuntu 20.04.4 LTS Display server: Xorg

[Photon89]

Well, if the comma is a valid character in a filename, and it seems to be, we just need to remove its char code, the 44, from https://github.com/shutter-project/shutter/blob/fab00c52f797fda35971ed50ac58b5c9226e8d42/bin/shutter#L1084 @DarthGandalf @ruzhnikov Any objections?

[DarthGandalf]

Why does it need to check some symbols there at all, instead of trusting what user wanted to do? :(

[Photon89]

Well, I traced it back to at least September 2010, I guess, Mario, the original developer, didn't trust users that much.

[vadi2]

I don't think we should trust the users to provide valid input.

[Photon89]

I think, we should first try to figure out, if Shutter itself runs into problems, if any of these characters are used.

[vadi2]

That would be a good approach

[Photon89]

So first of all, here is the list of char codes involved:

47  /
92  \
63  ?
42  *
58  :
124 |
34  "
60  <
62  >
44  ,
59  ;
35  #
38  &

We for sure should keep out the slash (47) and possibly also the backslash (92, it could lead to problems when copying the files into Windows machines). All other characters seem to be fine, at least I could use an upload plugin (Imgur guest), save the file in Shutter's editor, export it as a PDF file and apply a plugin (I tried Sepia) to it.

By the way, the list is defined on two occasions, not just the one I linked.

[DarthGandalf]

1. yes, let's remove everything from the list except those two
2. filtering it on keypress is not enough, it's possible to paste the comma or the slash from clipboard. This validation is better done when clicking the confirmation button instead of while typing

[DarthGandalf]

By the way, the list is defined on two occasions

the whole shutter code is full of duplicates

[Photon89]

I think, the direct check on typing is quite nice, too, maybe keep it and add another one on confirming? Also, maybe make the array of invalid char codes a global variable and define it only once?

[DarthGandalf]

That whole code of checking the keys etc needs to be deduped, and then there will be only 1 usage of such array, so it won't need to be global.

[Photon89]

@DarthGandalf

That whole code of checking the keys etc needs to be deduped, and then there will be only 1 usage of such array, so it won't need to be global.

Done!

filtering it on keypress is not enough, it's possible to paste the comma or the slash from clipboard. This validation is better done when clicking the confirmation button instead of while typing

I need some help here. I think, it would be the easiest to change the "key-press-event" to some kind of "on-change" event, that is, it should evaluate the current text field entry whenever it is changed. However, I don't know if such an event exist and if so, where to find corresponding documentation.

[DarthGandalf]

From https://docs.gtk.org/gtk3/class.Entry.html

changed or insert-text seem to be the most relevant

[Photon89]

Yep, I found those, but if in

$filename->signal_connect(
            'key-press-event' => sub {
                ...
            });

I substitute key-press-event with changed or insert-text, no input validation happens at all (neither on pasting nor on typing), so it looks like those are not available in Perl...

[DarthGandalf]

This works fine:

diff --git a/bin/shutter b/bin/shutter
index 8fc4bfb3..bc1f867b 100755
--- a/bin/shutter
+++ b/bin/shutter
@@ -9265,6 +9265,9 @@ sub STARTUP {
                        $new_filename_hint->set_markup("<span size='small'></span>");
                        return FALSE;
                    }
+               }) if 0;
+               $new_filename->signal_connect('changed' => sub {
+                   print "AAAAAAAAAAAAAAAAAA\n";
                });

            #parse filename

The signals have different behavior, you can't just replace one name with another.

[Photon89]

I don't understand, the lines added by you have the same structure as the original one? Maybe you better take it from here? :smiley: