Fix review comments

docs/news.d/1002.feature.rst

@@ -1,11 +1,11 @@
-[GHDL/NVC] Arbitrary waveform viewers are now supported by passing the `--viewer`
-command line arugment. As a consequence, ``ghdl.gtkwave_script.gui`` and
+[GHDL/NVC] Arbitrary waveform viewers are now supported by passing the ``--viewer``
+command line argument. As a consequence, ``ghdl.gtkwave_script.gui`` and
 ``nvc.gtkwave_script.gui`` are deprecated in favour of ``ghdl.viewer_script.gui``
 and ``nvc.viewer_script.gui``, respectively. The ``--gtkwave-args`` and
-``--gtkwave-fmt`` command line argument is deprecated in favour of ``--viewer-args``
+``--gtkwave-fmt`` command line arguments are deprecated in favour of ``--viewer-args``
 and ``--viewer-fmt``, respectively. ``ghdl.viewer.gui`` and ``nvc.viewer.gui`` can
-be used to set the preferred viewer from the run-file. Finally, if the requested
-viewer does not exists, ``gtkwave`` or ``surfer`` is used, in that order. This
-also means that VUnit uses ``surfer`` if ``gtkwave`` is not installed.
+be used to set the preferred viewer from the run-file. If no viewer is explicitly
+requested, ``gtkwave`` or ``surfer`` is used, in that order. This also means that
+VUnit now uses ``surfer`` if ``gtkwave`` is not installed.
 
 [NVC] It is possible to get VCD waveform files by passing ``--viewer-fmt=vcd``.

vunit/sim_if/_ossmixin.py → vunit/sim_if/_viewermixin.py

@@ -8,7 +8,7 @@
 """
 
 
-class OSSMixin:
+class ViewerMixin:
     """
     Mixin class for handling common viewer functionality for the GHDL and NVC simulators.
     """
@@ -35,8 +35,7 @@ def _get_viewer(self, config):
         """
         Determine the waveform viewer to use.
 
-        Falls back to gtkwave or surfer, in that order, if none is provided or the provided
-        cannot be found.
+        Falls back to gtkwave or surfer, in that order, if none is provided.
         """
         viewer = self._viewer or config.sim_options.get(self.name + ".viewer.gui", None)
 

vunit/sim_if/ghdl.py

@@ -21,12 +21,12 @@
 from ..ostools import Process
 from . import SimulatorInterface, ListOfStringOption, StringOption, BooleanOption
 from ..vhdl_standard import VHDL
-from ._ossmixin import OSSMixin
+from ._viewermixin import ViewerMixin
 
 LOGGER = logging.getLogger(__name__)
 
 
-class GHDLInterface(SimulatorInterface, OSSMixin):  # pylint: disable=too-many-instance-attributes
+class GHDLInterface(SimulatorInterface, ViewerMixin):  # pylint: disable=too-many-instance-attributes
     """
     Interface for GHDL simulator
     """
@@ -100,7 +100,7 @@ def __init__(  # pylint: disable=too-many-arguments
         backend="llvm",
     ):
         SimulatorInterface.__init__(self, output_path, gui)
-        OSSMixin.__init__(
+        ViewerMixin.__init__(
             self,
             gui=gui,
             viewer=viewer,
@@ -132,7 +132,7 @@ def _get_version_output(cls, prefix):
     @classmethod
     def _get_help_output(cls, prefix):
         """
-        Get the output of 'ghdl --version'
+        Get the output of 'ghdl --help'
         """
         return subprocess.check_output([str(Path(prefix) / cls.executable), "--help"]).decode()
 
@@ -392,11 +392,9 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only):  # pyl
             status = False
 
         if config.sim_options.get(self.name + ".gtkwave_script.gui", None):
-            warn_str = (
-                "%s.gtkwave_script.gui is deprecated and will be removed " % self.name  # pylint: disable=C0209
-                + "in a future version, use %s.viewer_script.gui instead" % self.name   # pylint: disable=C0209
-            )
-            LOGGER.warning(warn_str)
+            LOGGER.warning("%s.gtkwave_script.gui is deprecated and will be removed "
+                           "in a future version, use %s.viewer_script.gui instead",
+                           self.name, self.name)
 
         if self._gui and not elaborate_only:
             cmd = [self._get_viewer(config)] + shlex.split(self._viewer_args) + [data_file_name]

vunit/sim_if/nvc.py

@@ -20,13 +20,13 @@
 from ..ostools import Process
 from . import SimulatorInterface, ListOfStringOption, StringOption
 from . import run_command
-from ._ossmixin import OSSMixin
+from ._viewermixin import ViewerMixin
 from ..vhdl_standard import VHDL
 
 LOGGER = logging.getLogger(__name__)
 
 
-class NVCInterface(SimulatorInterface, OSSMixin):  # pylint: disable=too-many-instance-attributes
+class NVCInterface(SimulatorInterface, ViewerMixin):  # pylint: disable=too-many-instance-attributes
     """
     Interface for NVC simulator
     """
@@ -80,7 +80,7 @@ def __init__(  # pylint: disable=too-many-arguments
         if viewer_fmt == "ghw":
             LOGGER.warning("NVC does not support ghw, defaulting to fst")
             viewer_fmt = None  # Defaults to FST later
-        OSSMixin.__init__(self, gui=gui, viewer=viewer, viewer_fmt=viewer_fmt, viewer_args=viewer_args)
+        ViewerMixin.__init__(self, gui=gui, viewer=viewer, viewer_fmt=viewer_fmt, viewer_args=viewer_args)
 
         self._prefix = prefix
         self._project = None
@@ -305,11 +305,9 @@ def simulate(
             status = False
 
         if config.sim_options.get(self.name + ".gtkwave_script.gui", None):
-            warn_str = (
-                "%s.gtkwave_script.gui is deprecated and will be removed " % self.name  # pylint: disable=C0209
-                + "in a future version, use %s.viewer_script.gui instead" % self.name   # pylint: disable=C0209
-            )
-            LOGGER.warning(warn_str)
+            LOGGER.warning("%s.gtkwave_script.gui is deprecated and will be removed "
+                           "in a future version, use %s.viewer_script.gui instead",
+                           self.name, self.name)
 
         if self._gui and not elaborate_only:
             cmd = [self._get_viewer(config)] + shlex.split(self._viewer_args) + [str(wave_file)]