Merge pull request #21574 from ccordoba12/pythonpath-options

PR: Some improvements to the Pythonpath plugin

spyder/app/mainwindow.py

@@ -82,8 +82,7 @@
 from spyder.py3compat import to_text_string
 from spyder.utils import encoding, programs
 from spyder.utils.icon_manager import ima
-from spyder.utils.misc import (select_port, getcwd_or_home,
-                               get_python_executable)
+from spyder.utils.misc import select_port, getcwd_or_home
 from spyder.utils.palette import QStylePalette
 from spyder.utils.qthelpers import file_uri, qapplication, start_file
 from spyder.utils.stylesheet import APP_STYLESHEET
@@ -1199,31 +1198,6 @@ def redirect_internalshell_stdio(self, state):
             else:
                 console.restore_stds()
 
-    def open_external_console(self, fname, wdir, args, interact, debug, python,
-                              python_args, systerm, post_mortem=False):
-        """Open external console"""
-        if systerm:
-            # Running script in an external system terminal
-            try:
-                if self.get_conf('default', section='main_interpreter'):
-                    executable = get_python_executable()
-                else:
-                    executable = self.get_conf(
-                        'executable',
-                        section='main_interpreter'
-                    )
-                pypath = self.get_conf('spyder_pythonpath', default=None,
-                                       section='pythonpath_manager')
-                programs.run_python_script_in_terminal(
-                    fname, wdir, args, interact, debug, python_args,
-                    executable, pypath
-                )
-            except NotImplementedError:
-                QMessageBox.critical(self, _("Run"),
-                                     _("Running an external system terminal "
-                                       "is not supported on platform %s."
-                                       ) % os.name)
-
     def open_file(self, fname, external=False):
         """
         Open filename with the appropriate application

spyder/app/tests/test_mainwindow.py

@@ -6353,34 +6353,70 @@ def test_PYTHONPATH_in_consoles(main_window, qtbot, tmp_path,
     qtbot.waitUntil(lambda: shell._prompt_html is not None,
                     timeout=SHELL_TIMEOUT)
 
-    # Add a new directory to PYTHONPATH
-    new_dir = tmp_path / 'new_dir'
-    new_dir.mkdir()
-    set_user_env({"PYTHONPATH": str(new_dir)})
-
-    # Open Pythonpath dialog to detect new_dir
+    # Main variables
     ppm = main_window.get_plugin(Plugins.PythonpathManager)
+
+    # Add a directory to PYTHONPATH
+    sys_dir = tmp_path / 'sys_dir'
+    sys_dir.mkdir()
+    set_user_env({"PYTHONPATH": str(sys_dir)})
+
+    # Add a directory to the current list of paths to simulate a path added by
+    # users
+    user_dir = tmp_path / 'user_dir'
+    user_dir.mkdir()
+    if os.name != "nt":
+        assert ppm.get_container().path == ()
+    ppm.get_container().path = (str(user_dir),) + ppm.get_container().path
+
+    # Open Pythonpath dialog to detect sys_dir
     ppm.show_path_manager()
     qtbot.wait(500)
 
-    # Check new_dir was added to sys.path after closing the dialog
-    ppm.path_manager_dialog.close()
+    # Check we're showing two headers
+    assert len(ppm.path_manager_dialog.headers) == 2
+
+    # Check the PPM emits the right signal after closing the dialog
+    with qtbot.waitSignal(ppm.sig_pythonpath_changed, timeout=1000):
+        ppm.path_manager_dialog.close()
+
+    # Check directories were added to sys.path in the right order
     with qtbot.waitSignal(shell.executed, timeout=2000):
         shell.execute("import sys; sys_path = sys.path")
 
-    assert str(new_dir) in shell.get_value("sys_path")
+    sys_path = shell.get_value("sys_path")
+    assert sys_path[-2:] == [str(user_dir), str(sys_dir)]
 
     # Create new console
     ipyconsole.create_new_client()
-    shell = ipyconsole.get_current_shellwidget()
-    qtbot.waitUntil(lambda: shell._prompt_html is not None,
+    shell1 = ipyconsole.get_current_shellwidget()
+    qtbot.waitUntil(lambda: shell1._prompt_html is not None,
                     timeout=SHELL_TIMEOUT)
 
-    # Check new_dir is part of the new console's sys.path
-    with qtbot.waitSignal(shell.executed, timeout=2000):
-        shell.execute("import sys; sys_path = sys.path")
+    # Check directories are part of the new console's sys.path
+    with qtbot.waitSignal(shell1.executed, timeout=2000):
+        shell1.execute("import sys; sys_path = sys.path")
+
+    sys_path = shell1.get_value("sys_path")
+    assert sys_path[-2:] == [str(user_dir), str(sys_dir)]
+
+    # Check that disabling a path from the PPM removes it from sys.path in all
+    # consoles
+    ppm.show_path_manager()
+    qtbot.wait(500)
+
+    item = ppm.path_manager_dialog.listwidget.item(1)
+    item.setCheckState(Qt.Unchecked)
+
+    with qtbot.waitSignal(ppm.sig_pythonpath_changed, timeout=1000):
+        ppm.path_manager_dialog.accept()
+
+    for s in [shell, shell1]:
+        with qtbot.waitSignal(s.executed, timeout=2000):
+            s.execute("import sys; sys_path = sys.path")
 
-    assert str(new_dir) in shell.get_value("sys_path")
+        sys_path = s.get_value("sys_path")
+        assert str(user_dir) not in sys_path
 
 
 @flaky(max_runs=10)

spyder/plugins/externalterminal/plugin.py

@@ -229,18 +229,27 @@ def on_editor_teardown(self):
     # ---- Public API
     # -------------------------------------------------------------------------
     def open_external_python_terminal(self, fname, wdir, args, interact, debug,
-                                     python_args):
+                                      python_args):
         """Open external terminal."""
         # Running script in an external system terminal
         try:
             if self.get_conf('default', section='main_interpreter'):
                 executable = get_python_executable()
             else:
-                executable = self.get_conf('executable',
-                                           section='main_interpreter')
+                executable = self.get_conf(
+                    'executable',
+                    section='main_interpreter'
+                )
+
+            pypath = self.get_conf(
+                'spyder_pythonpath',
+                section='pythonpath_manager',
+                default=[]
+            )
 
             programs.run_python_script_in_terminal(
-                fname, wdir, args, interact, debug, python_args, executable
+                fname, wdir, args, interact, debug, python_args, executable,
+                pypath=pypath
             )
         except NotImplementedError:
             QMessageBox.critical(
@@ -269,7 +278,8 @@ def run_python_files(
         debug = False
         python_args = params['python_args']
         self.open_external_python_terminal(
-            filename, wdir, args, interact, debug, python_args)
+            filename, wdir, args, interact, debug, python_args
+        )
 
     @run_execute(extension=['sh', 'bat', 'ps1'])
     def run_shell_files(

spyder/plugins/ipythonconsole/tests/test_ipythonconsole.py

@@ -1991,6 +1991,7 @@ def get_cwd_of_new_client():
     assert get_cwd_of_new_client() == fixed_dir
 
 
+@flaky(max_runs=10)
 def test_startup_run_lines_project_directory(ipyconsole, qtbot, tmpdir):
     """
     Test 'startup/run_lines' config works with code from an active project.

spyder/plugins/pythonpath/container.py

@@ -18,7 +18,6 @@
 from spyder.config.base import get_conf_path
 from spyder.plugins.pythonpath.utils import get_system_pythonpath
 from spyder.plugins.pythonpath.widgets.pathmanager import PathManager
-from spyder.utils import encoding
 
 
 # Logging
@@ -35,9 +34,6 @@ class PythonpathActions:
 # -----------------------------------------------------------------------------
 class PythonpathContainer(PluginMainContainer):
 
-    PATH_FILE = get_conf_path('path')
-    NOT_ACTIVE_PATH_FILE = get_conf_path('not_active_path')
-
     sig_pythonpath_changed = Signal(object, object)
 
     def __init__(self, *args, **kwargs):
@@ -50,6 +46,10 @@ def __init__(self, *args, **kwargs):
     # -------------------------------------------------------------------------
     def setup(self):
 
+        # Migrate from old conf files to config options
+        if self.get_conf('paths_in_conf_files', default=True):
+            self._migrate_to_config_options()
+
         # Load Python path
         self._load_pythonpath()
 
@@ -141,31 +141,25 @@ def _load_pythonpath(self):
         previous_system_path = self.get_conf('system_path', default=())
 
         # Load all paths
-        if osp.isfile(self.PATH_FILE):
-            with open(self.PATH_FILE, 'r', encoding='utf-8') as f:
-                previous_paths = f.read().splitlines()
+        paths = []
+        previous_paths = self.get_conf('path')
+        for path in previous_paths:
+            # Path was removed since last time or it's not a directory
+            # anymore
+            if not osp.isdir(path):
+                continue
 
-            paths = []
-            for path in previous_paths:
-                # Path was removed since last time or it's not a directory
-                # anymore
-                if not osp.isdir(path):
-                    continue
+            # Path was removed from system path
+            if path in previous_system_path and path not in system_path:
+                continue
 
-                # Path was removed from system path
-                if path in previous_system_path and path not in system_path:
-                    continue
+            paths.append(path)
 
-                paths.append(path)
+        self.path = tuple(paths)
 
-            self.path = tuple(paths)
-
-        # Update path file contents. This avoids loading paths that were
-        # removed in this session in later ones.
-        try:
-            encoding.writelines(self.path, self.PATH_FILE)
-        except OSError as e:
-            logger.error(str(e))
+        # Update path option. This avoids loading paths that were removed in
+        # this session in later ones.
+        self.set_conf('path', self.path)
 
         # Update system path so that path_manager_dialog can work with its
         # latest contents.
@@ -176,34 +170,32 @@ def _load_pythonpath(self):
             self.path = self.path + system_path
 
         # Load not active paths
-        if osp.isfile(self.NOT_ACTIVE_PATH_FILE):
-            with open(self.NOT_ACTIVE_PATH_FILE, 'r', encoding='utf-8') as f:
-                not_active_path = f.read().splitlines()
-            self.not_active_path = tuple(name for name in not_active_path
-                                         if osp.isdir(name))
+        not_active_paths = self.get_conf('not_active_path')
+        self.not_active_path = tuple(
+            name for name in not_active_paths if osp.isdir(name)
+        )
 
-    def _save_python_path(self, new_path_dict):
+    def _save_paths(self, new_path_dict):
         """
-        Save Spyder PYTHONPATH to configuration folder and update attributes.
+        Save tuples for all paths and not active ones to config system and
+        update their associated attributes.
 
         `new_path_dict` is an OrderedDict that has the new paths as keys and
         the state as values. The state is `True` for active and `False` for
         inactive.
         """
         path = tuple(p for p in new_path_dict)
-        not_active_path = tuple(p for p in new_path_dict
-                                if not new_path_dict[p])
-
-        if path != self.path or not_active_path != self.not_active_path:
-            # Do not write unless necessary
-            try:
-                encoding.writelines(path, self.PATH_FILE)
-                encoding.writelines(not_active_path,
-                                    self.NOT_ACTIVE_PATH_FILE)
-            except OSError as e:
-                logger.error(str(e))
+        not_active_path = tuple(
+            p for p in new_path_dict if not new_path_dict[p]
+        )
 
+        # Don't set options unless necessary
+        if path != self.path:
+            self.set_conf('path', path)
             self.path = path
+
+        if not_active_path != self.not_active_path:
+            self.set_conf('not_active_path', not_active_path)
             self.not_active_path = not_active_path
 
     def _get_spyder_pythonpath_dict(self):
@@ -239,7 +231,7 @@ def _update_python_path(self, new_path_dict=None):
 
         # Save new path
         if new_path_dict is not None:
-            self._save_python_path(new_path_dict)
+            self._save_paths(new_path_dict)
 
         # Load new path plus project path
         new_path_dict_p = self._get_spyder_pythonpath_dict()
@@ -250,3 +242,27 @@ def _update_python_path(self, new_path_dict=None):
             logger.debug(f"Update Pythonpath to {pypath}")
             self.set_conf('spyder_pythonpath', pypath)
             self.sig_pythonpath_changed.emit(old_path_dict_p, new_path_dict_p)
+
+    def _migrate_to_config_options(self):
+        """
+        Migrate paths saved in the `path` and `not_active_path` files located
+        in our config directory to our config system.
+
+        This was the way we save those paths in Spyder 5 and before.
+        """
+        path_file = get_conf_path('path')
+        not_active_path_file = get_conf_path('not_active_path')
+
+        path = []
+        if osp.isfile(path_file):
+            with open(path_file, 'r', encoding='utf-8') as f:
+                path = f.read().splitlines()
+
+        not_active_path = []
+        if osp.isfile(not_active_path_file):
+            with open(not_active_path_file, 'r', encoding='utf-8') as f:
+                not_active_path = f.read().splitlines()
+
+        self.set_conf('path', tuple(path))
+        self.set_conf('not_active_path', tuple(not_active_path))
+        self.set_conf('paths_in_conf_files', False)

spyder/plugins/pythonpath/plugin.py

@@ -50,11 +50,6 @@ class PythonpathManager(SpyderPluginV2):
     new_path_dict: OrderedDict
         New Pythonpath dictionary.
 
-    Notes
-    -----
-    It should be possible to simplify this by sending only the new path dict.
-    However, that requires changes in Spyder-kernels.
-
     See Also
     --------
     :py:meth:`.PythonpathContainer._get_spyder_pythonpath_dict`