Fixed error when dialog choice is cancelled

[SpookedByRoaches]

Clicking cancel on the dialog menu would cause 2 things.
1- It would make the return_code variable nonzero
2- It would make the result a non integer

So cancelling the choice would cause an error on the script. This fix would check the return code and return None on any nonzero return code.

[josephj11]

Nice catch!

I'm surprised we didn't run into this before. There are several valid non-zero return codes when the user presses ESC, makes no choice, or just closes the dialog window.

LGTM.

Someone else should test this before it gets merged. @Elliria ?

I took a look at dialog_gtk.py. It's different enough that I can't immediatiely tell if it has a similar problem, but that should be tested as well.

[Elliria]

Interesting.

Part 1 of testing

Radio lists fall over in some cases in AutoKey 0.95.10:

They work fine if the result is printed to a window:

• If the result is printed to the currently-active window by the keyboard.send_keys() API call and I press the Cancel button after making a choice, the cancel message is printed instantly.
• If the result is printed to the currently-active window by the keyboard.send_keys() API call and I press the Cancel button without making a choice, the cancel message is printed instantly.

# Define the list:
choices = ["apple", "banana", "coconut"]

# Offer the list choices in a dialog and check the return code of the command:
retCode, choice = dialog.list_menu(choices)

# Define the messages:
this_choice_message = "You chose " + choice + "."
no_choice_message = "No choice was made."
cancel_message = "You cancelled."

# If the return code (exit code) is zero:
if retCode == 0:
    # If the length of the choice is greater than zero characters:
    if len(choice)>0:
        # Print the result to the currently-active window:
        keyboard.send_keys(this_choice_message)
    # If the length of the choice is zero characters:
    else:
        # Print the result to the currently-active window:
        keyboard.send_keys(no_choice_message)
# If the return code (exit code) is not zero:
else:
        # Print the result to the currently-active window:
        keyboard.send_keys(cancel_message)

They fall over if the result is displayed in a dialog:

• If the result is configured to be displayed in a dialog and I press the Cancel button after making a choice, the radio-list dialog remains on the screen and nothing else happens. If I then close the radio-list dialog by clicking the X in the top right corner of its window, my cancel dialog is displayed. This is bad behavior since the radio list should have vanished and the cancel dialog should have taken its place.
• If the result is configured to be displayed in a dialog and I press the Cancel button without making a choice, the cancel message is displayed instantly in a dialog.

# Define the list:
choices = ["apple", "banana", "coconut"]

# Offer the list choices in a dialog and check the return code of the command:
retCode, choice = dialog.list_menu(choices)

# Define the messages:
this_choice_message = "You chose " + choice + "."
no_choice_message = "No choice was made."
cancel_message = "You cancelled."

# If the return code (exit code) is zero:
if retCode == 0:
    # If the length of the choice is greater than zero characters:
    if len(choice)>0:
        # Display the result in a dialog:
        dialog.info_dialog(title="Result", message=this_choice_message)
    # If the length of the choice is zero characters:
    else:
        # Display the result in a dialog:
        dialog.info_dialog(title="Result", message=no_choice_message)
# If the return code (exit code) is not zero:
else:
        # Display the result in a dialog:
        dialog.info_dialog(title="Result", message=cancel_message)

Part 2 of testing

I didn't clone the PR because it's in a fork, so I made the modification to the file manually in a cloned version of the development code, which is at version 0.96.0-beta.9.

I'm not succeeding with testing this so far. The above code examples can't be used to test this because they rely on an empty string to determine when nothing has been chosen. So far, all of my attempts to write a test that checks either for a value of None or a type of type(None) or even something as basic as displaying the output in a dialog have all failed.

If either of you have (or anyone has) a script I can try that works with the new None type, I'd be happy to test it. For now, I'm going to quit messing with it, because everything I try is ending up in errors.

[josephj11]

The defensive programming of your test examples prevents the reported bug from occurring. The proposed PR is to prevent an exception being thrown, but your test code without the PR does not throw an exception - which means it is probably not testing the original problem. It appears that what you have discovered is an additional bug or additional aspect of the same bug.

You should also test when selecting or not and then pressing ESC.

The failure you describe is very weird. I have used yad a lot (Zenity fork) and have never seen pressing the cancel button fail to dismiss the dialog window.

Are we sure this has anything to do with the particular kdialog dialog type? This seems like a commmon feature of kdialog. It should work or not work the same in every kdialog dialog which offers a Cancel button. It may also be present in standalone kdialogs - which would rule out AutoKey as the cause.

Since autokey-gtk uses Zenity, all that has to be tested separately.

[SpookedByRoaches]

@Elliria
That's strange. I've tested your code and it fails before even entering the part where it makes a dialog or prints to the window. When I test the code without my PR, I get this error.

raceback (most recent call last):
  File "/home/REDACTED/.local/lib/python3.11/site-packages/autokey/service.py", line 530, in _execute
    exec(compiled_code, scope)
  File "/home/REDACTED/.config/autokey/data/Sample Scripts/One from GH.py", line 5, in <module>
    retCode, choice = dialog.list_menu(choices)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/REDACTED/.local/lib/python3.11/site-packages/autokey/scripting/dialog_qt.py", line 130, in list_menu
    choice = options[int(result)]
                     ^^^^^^^^^^^
ValueError: invalid literal for int() with base 10: ''

But when I try it with my edit I get this

Traceback (most recent call last):
  File "/home/REDACTED/.local/lib/python3.11/site-packages/autokey/service.py", line 530, in _execute
    exec(compiled_code, scope)
  File "/home/REDACTED/.config/autokey/data/Sample Scripts/One from GH.py", line 8, in <module>
    this_choice_message = "You chose " + choice + "."
                          ~~~~~~~~~~~~~^~~~~~~~
TypeError: can only concatenate str (not "NoneType") to str

I don't know if something is wrong with my specific installation but if it helps, I'm using the AUR version.

[Elliria]

Successful tests

I've now managed to test it properly and it passes muster. This is a good fix. I'd recommend putting this PR through.

The script:

# Define the list:
choices = ["apple", "banana", "coconut"]

# Offer the list choices in a dialog and check the return code of the command:
retCode, choice = dialog.list_menu(choices)

# If the return code (exit code) is zero:
if retCode == 0:
    # If the length of the choice is greater than zero characters:
    if len(choice) == 0:
        # Display the result in a dialog:
        dialog.info_dialog(title="Result", message="No choice was made.")
    else:
        # Display the result in a dialog:
        dialog.info_dialog(title="Result", message="You chose " + choice + ".")
# If the return code (exit code) is not zero:
else:
    # Display the result in a dialog:
    dialog.info_dialog(title="Result", message="You cancelled.")

The tests:

1. Press OK after making a choice
2. Press OK without making a choice.
3. Press Cancel after making a choice.
4. Press Cancel without making a choice.

The test results:

Test 1 - AutoKey 0.95.10 GTK 💚:

1. Dialog: "You chose apple."
2. Dialog: "No choice was made."
3. Dialog: "You cancelled."
4. Dialog: "You cancelled."

Test1-autokey-gtk-0.95.10.log

Test 2 - AutoKey 0.95.10 Qt result 🛑:

1. Dialog: "You chose apple."
2. Dialog: "You chose apple."
3. AutoKey Error - see log
4. AutoKey Error - see log

Test2-autokey-qt-0.95.10.log

Test 3 - AutoKey 0.96.0 GTK result 💚:

1. Dialog: "You chose apple."
2. Dialog: "No choice was made."
3. Dialog: "You cancelled."
4. Dialog: "You cancelled."

Test3-autokey-gtk-0.96.0.log

Test 4 - AutoKey 0.96.0 Qt result 🛑:

1. Dialog: "You chose apple."
2. Dialog: "You chose apple."
3. AutoKey Error - see log
4. AutoKey Error - see log

Test4-autokey-qt-0.96.0.log

Test 5 - AutoKey 0.96.0 GTK result - WITH the fix 💚:

1. Dialog: "You chose apple."
2. Dialog: "No choice was made."
3. Dialog: "You cancelled."
4. Dialog: "You cancelled."

Test5-autokey-gtk-0.96.0-WithFix.log

Test 6 - AutoKey 0.96.0 Qt result - WITH the fix 💚:

1. Dialog: "You chose apple."
2. Dialog: "You chose apple."
3. Dialog: "You cancelled."
4. Dialog: "You cancelled."

Test6-autokey-qt-0.96.0-WithFix.log

Notes

• As part of my testing, when I applied the fix, I tried choice = None and I also tried choice = "" as the fix. Both performed equally well, so either could be used here. Then again, I didn't test the use of integers as radio-list values, so the None value seems to be the best choice.
• AutoKey was run from the command line in all tests. The commands used are in the logs.
• AutoKey 0.95.10 was installed from the Ubuntu repository.
• AutoKey 0.96.0 was cloned from the development version of AutoKey.
• AutoKey 0.96.0 was run in a virtual environment (venv).
• Although no changes were made to the GTK code, all tests were also done with autokey-gtk as a comparison.
• The autokey-gtk code doesn't require a fix.
• In Qt, the first radio-list item is always chosen for you by default, so there's no such thing as choosing nothing from the list. Hence, it always said this when no choice was made: "You chose apple." This was not an error, but is simply a default KDialog behavior that we don't have control over, so if we wanted to use the script I used for testing in autokey-qt, we would remove the nested conditional block and simply always output the choice whenever the OK button is pressed. The code would then look like this instead:

  # Define the list:
  choices = ["apple", "banana", "coconut"]
  
  # Offer the list choices in a dialog and check the return code of the command:
  retCode, choice = dialog.list_menu(choices)
  
  # If the return code (exit code) is zero:
  if retCode == 0:
      # Display the result in a dialog:
      dialog.info_dialog(title="Result", message="You chose " + choice + ".")
  # If the return code (exit code) is not zero:
  else:
      # Display the result in a dialog:
      dialog.info_dialog(title="Result", message="You cancelled.")

[josephj11]

@SpookedByRoaches Thanks for this fix!

@Elliria Thanks for the extensive testing.

Can you look into removing the Python 3.7 tests (end of life?) and when 3.11 ... should be added?

[Elliria]

Can you look into removing the Python 3.7 tests (end of life?) and when 3.11 ... should be added?

This will require a discussion so we can sort out the version range to choose, so I'll open one for it.