Repository: spyder-ide/spyder
Pull request: PR: Clear old set shortcuts if they are now empty (Shortcuts)
<!--- Make sure to read the Contributing Guidelines:                   --->
<!--- https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md --->
<!--- and follow PEP 8, PEP 257 and Spyder's code style:               --->
<!--- https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style      --->

## Description of Changes


* [ ] Wrote at least one-line docstrings (for any new functions)
* [ ] Added unit test(s) covering the changes (if testable)
<!--- Remember that an image/animation is worth a thousand words! --->
* [ ] Included a screenshot or animation (if affecting the UI, see [Licecap](https://www.cockos.com/licecap/))


<!--- Explain what you've done and why --->

If a shortcut conflict is detected, and the user decides to do an overwrite of a shortcut, the old action needs to unset the shortcut to prevent having two actions with the same shortcut.

### Issue(s) Resolved

<!--- List the issue(s) below, in the form "Fixes #1234"; one per line --->

Fixes #12992 


### Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the [Developer Certificate of Origin](https://developercertificate.org)
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

<!--- TYPE YOUR USER/NAME AFTER THE FOLLOWING: --->
I certify the above statement is true and correct: dalthviz

<!--- Thanks for your help making Spyder better for everyone! --->


Problem statement:
Shift+return not working for run selection -  no solutions work
## Problem Description
Updated to most recent Spyder version (4.1.3). Wanted to change the "run selection" back to what I was used to in previous version, shift+return. So: preferences > "run selection" > pressed Shift+Return in new shortcut. Apply, Ok. 
Tried to run a selection with Shift+Return, nothing happens. The "run selection" button does work normally. Also tried to run it with the "enter" button on my numpad, doesn't work either. Restarted Spyder, used --reset, no changes. NOTE: if I change the "run selection" to something else (e.g. Shift+Backspace) it does work as expected.

### What steps reproduce the problem?
1. Update Spyder to 4.1.3
2. Set "run selection" to Shift+Return
3. Try to run selection with Shift+Return

### What is the expected output? What do you see instead?
Expected: to run the selection
Instead: nothing happens

## Versions
* Spyder version: 4.1.3
* Python version: 3.7
* Operating System name/version: windows

Thanks for this otherwise wonderful software!

Hints from issue discussion:
Hi @EvaNewzoo I was able to reproduce this! It seems like changes in shortcuts that are detected as conflicts are not being set correctly if the old shortcut is not set to something else too or you restart Spyder:

![image](https://user-images.githubusercontent.com/16781833/84294120-53ad9c80-ab0e-11ea-94c2-8ab3f680c079.png)

As a workaround, could you try to redefine the shortcut for the `run selection` and also `run cell and advance` or restart Spyder after the shortcut change is applied? 

We will fix this in a future release :)

I think we should try to solve this for 4.1.4, if it's not that hard to do. What do you think?
Sure :+1:
Ok, please work on this one then.
Thanks, much appreciated!

Commit message: Shortcuts: Skip empty shortcut test on MacOS

Patch to review:
diff --git a/spyder/app/mainwindow.py b/spyder/app/mainwindow.py
index fa7f232ae31..10a7e790582 100644
--- a/spyder/app/mainwindow.py
+++ b/spyder/app/mainwindow.py
@@ -3258,23 +3258,27 @@ def apply_shortcuts(self):
 
             if shortcut_sequence:
                 keyseq = QKeySequence(shortcut_sequence)
-                try:
-                    if isinstance(qobject, QAction):
-                        if (sys.platform == 'darwin'
-                                and qobject._shown_shortcut == 'missing'):
-                            qobject._shown_shortcut = keyseq
-                        else:
-                            qobject.setShortcut(keyseq)
+            else:
+                # Needed to remove old sequences that were cleared.
+                # See spyder-ide/spyder#12992
+                keyseq = QKeySequence()
+            try:
+                if isinstance(qobject, QAction):
+                    if (sys.platform == 'darwin'
+                            and qobject._shown_shortcut == 'missing'):
+                        qobject._shown_shortcut = keyseq
+                    else:
+                        qobject.setShortcut(keyseq)
 
-                        if add_shortcut_to_tip:
-                            add_shortcut_to_tooltip(qobject, context, name)
+                    if add_shortcut_to_tip:
+                        add_shortcut_to_tooltip(qobject, context, name)
 
-                    elif isinstance(qobject, QShortcut):
-                        qobject.setKey(keyseq)
+                elif isinstance(qobject, QShortcut):
+                    qobject.setKey(keyseq)
 
-                except RuntimeError:
-                    # Object has been deleted
-                    toberemoved.append(index)
+            except RuntimeError:
+                # Object has been deleted
+                toberemoved.append(index)
 
         for index in sorted(toberemoved, reverse=True):
             self.shortcut_data.pop(index)
diff --git a/spyder/app/tests/test_mainwindow.py b/spyder/app/tests/test_mainwindow.py
index b25e5c5732a..e24ba856e1b 100644
--- a/spyder/app/tests/test_mainwindow.py
+++ b/spyder/app/tests/test_mainwindow.py
@@ -2421,6 +2421,60 @@ def test_preferences_change_font_regression(main_window, qtbot):
                     timeout=5000)
 
 
+@pytest.mark.slow
+@pytest.mark.skipif(
+    sys.platform == 'darwin',
+    reason="Changes of Shitf+Return shortcut cause an ambiguos shortcut")
+def test_preferences_empty_shortcut_regression(main_window, qtbot):
+    """
+    Test for spyder-ide/spyder/#12992 regression.
+
+    Overwritting shortcuts results in a shortcuts conflict.
+    """
+    # Wait until the window is fully up
+    shell = main_window.ipyconsole.get_current_shellwidget()
+    qtbot.waitUntil(lambda: shell._prompt_html is not None,
+                    timeout=SHELL_TIMEOUT)
+
+    # Setup shortcuts (set run cell and advance shortcut to run selection)
+    base_run_cell_advance = CONF.get_shortcut(
+        'editor', 'run cell and advance')  # Should be Shift+Return
+    base_run_selection = CONF.get_shortcut(
+        'editor', 'run selection')  # Should be F9
+    assert base_run_cell_advance == 'Shift+Return'
+    assert base_run_selection == 'F9'
+    CONF.set_shortcut(
+        'editor', 'run cell and advance', '')
+    CONF.set_shortcut(
+        'editor', 'run selection', base_run_cell_advance)
+    main_window.apply_shortcuts()
+
+    # Check execution of shortcut
+    # Create new file
+    main_window.editor.new()
+    code_editor = main_window.editor.get_focus_widget()
+    code_editor.set_text('print(0)\nprint(1)\nprint(2)')
+
+    with qtbot.waitSignal(shell.executed):
+        qtbot.keyClick(code_editor, Qt.Key_Return, modifier=Qt.ShiftModifier)
+    qtbot.waitUntil(lambda: 'print(0)' in shell._control.toPlainText())
+    assert 'print(1)' not in shell._control.toPlainText()
+
+    # Reset shortcuts
+    CONF.set_shortcut(
+        'editor', 'run selection', 'F9')
+    CONF.set_shortcut(
+        'editor', 'run cell and advance', 'Shift+Return')
+    main_window.apply_shortcuts()
+    qtbot.wait(500)  # Wait for shortcut change to actually be applied
+
+    # Check shortcut run cell and advance reset
+    code_editor.setFocus()
+    with qtbot.waitSignal(shell.executed):
+        qtbot.keyClick(code_editor, Qt.Key_Return, modifier=Qt.ShiftModifier)
+    qtbot.waitUntil(lambda: 'runcell(0' in shell._control.toPlainText())
+
+
 @pytest.mark.slow
 def test_preferences_shortcut_reset_regression(main_window, qtbot):
     """


Code review comments:
diff_hunk: @@ -2421,6 +2421,60 @@ def test_preferences_change_font_regression(main_window, qtbot):
                     timeout=5000)
 
 
+@pytest.mark.slow
+@pytest.mark.skipif(
+    sys.platform == 'darwin',
+    reason="Changes of Shitf+Return shortcut cause an ambiguos shortcut")
+def test_preferences_empty_shortcut_regression(main_window, qtbot):
+    """
+    Test for spyder-ide/spyder/#12992 regression.
+
+    Overwritting shortcuts results in a shortcuts conflict.
+    """
+    # Wait until the window is fully up
+    shell = main_window.ipyconsole.get_current_shellwidget()
+    qtbot.waitUntil(lambda: shell._prompt_html is not None,
+                    timeout=SHELL_TIMEOUT)
+
+    # Setup shortcuts (set run cell and advance shortcut to run selection)
+    base_run_cell_advance = CONF.get_shortcut(
+        'editor', 'run cell and advance')  # Should be Shift+Return
+    base_run_selection = CONF.get_shortcut(
+        'editor', 'run selection')  # Should be F9
+    assert base_run_cell_advance == 'Shift+Return'
+    assert base_run_selection == 'F9'
+    CONF.set_shortcut(
+        'editor', 'run cell and advance', '')
+    CONF.set_shortcut(
+        'editor', 'run selection', base_run_cell_advance)
+    main_window.apply_shortcuts()
+
+    # Check execution of shortcut
+    # Create new file
+    main_window.editor.new()
+    code_editor = main_window.editor.get_focus_widget()
+    code_editor.set_text('print(0)\nprint(1)\nprint(2)')
+
+    with qtbot.waitSignal(shell.executed):
+        qtbot.keyClick(code_editor, Qt.Key_Return, modifier=Qt.ShiftModifier)
+    qtbot.waitUntil(lambda: 'print(0)' in shell._control.toPlainText())
+    assert 'print(1)' not in shell._control.toPlainText()
path: spyder/app/tests/test_mainwindow.py
text: @user1:
I think this is incorrect because this is what you get when you have the following code:

```python
print(0)
print(1)
print(2)
```

and press `Shift+Enter` (i.e. in case the shortcut was not changed correctly):

![Selección_013](https://user-images.githubusercontent.com/365293/85318593-df67e700-b485-11ea-9595-ce29cd694ba9.png)

As you can see, there's no `print(1)` there.

@user1:
So I'd suggest to change the text to be executed to

    code_editor.set_text("print(0)\nprint('ññ')")

and assert that `ññ` is not present in `shell._control.toPlainText()`. That's because there are lot of other numbers in the console (for instance in the banner on the top) that could match other print's with numbers.