Review feedback

src/mopidy_mpd/dispatcher.py

@@ -106,7 +106,7 @@ def handle_idle(self, subsystem: str) -> None:
         self.session.send_lines(response)
 
     def _call_next_filter(
-        self, request: str, response: Response, filter_chain: list[Filter]
+        self, request: Request, response: Response, filter_chain: list[Filter]
     ) -> Response:
         if filter_chain:
             next_filter = filter_chain.pop(0)
@@ -175,10 +175,10 @@ def _command_list_filter(
             response = Response(response[:-1])
         return response
 
-    def _is_receiving_command_list(self, request: str) -> bool:
+    def _is_receiving_command_list(self, request: Request) -> bool:
         return self.command_list_receiving and request != "command_list_end"
 
-    def _is_processing_command_list(self, request: str) -> bool:
+    def _is_processing_command_list(self, request: Request) -> bool:
         return self.command_list_index is not None and request != "command_list_end"
 
     # --- Filter: idle
@@ -243,7 +243,7 @@ def _call_handler_filter(
             logger.warning("Tried to communicate with dead actor.")
             raise exceptions.MpdSystemError(str(exc)) from exc
 
-    def _call_handler(self, request: str) -> protocol.Result:
+    def _call_handler(self, request: Request) -> protocol.Result:
         tokens = tokenize.split(request)
         # TODO: check that blacklist items are valid commands?
         blacklist = self.config["mpd"]["command_blacklist"]

src/mopidy_mpd/network.py

@@ -329,7 +329,7 @@ def send(self, data: bytes) -> bytes:
 
     def enable_timeout(self) -> None:
         """Reactivate timeout mechanism."""
-        if self.timeout is None or self.timeout <= 0:
+        if self.timeout <= 0:
             return
 
         self.disable_timeout()

src/mopidy_mpd/protocol/current_playlist.py

@@ -341,7 +341,7 @@ def plchangesposid(context: MpdContext, version: int) -> protocol.Result:
 
 
 @protocol.commands.add("prio", priority=protocol.UINT, position=protocol.RANGE)
-def prio(context: MpdContext, priority: int, position: int) -> protocol.Result:
+def prio(context: MpdContext, priority: int, position: slice) -> protocol.Result:
     """
     *musicpd.org, current playlist section:*
 

src/mopidy_mpd/protocol/music_db.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 import itertools
-from typing import TYPE_CHECKING, Never, cast
+from typing import TYPE_CHECKING, cast
 
 from mopidy.models import Album, Artist, SearchResult, Track
 from mopidy.types import DistinctField, Query, SearchField, Uri
@@ -360,7 +360,7 @@ def listallinfo(context: MpdContext, uri: str | None = None) -> protocol.Result:
 
 
 @protocol.commands.add("listfiles")
-def listfiles(context: MpdContext, uri: str | None = None) -> Never:
+def listfiles(context: MpdContext, uri: str | None = None) -> protocol.Result:
     """
     *musicpd.org, music database section:*
 
@@ -521,6 +521,11 @@ def searchaddpl(context: MpdContext, *args: str) -> None:
         raise exceptions.MpdArgError("incorrect arguments")
 
     playlist_name = parameters.pop(0)
+    try:
+        query = _query_for_search(parameters)
+    except ValueError:
+        return
+
     uri = context.uri_map.playlist_uri_from_name(playlist_name)
     if uri:
         playlist = context.core.playlists.lookup(uri).get()
@@ -529,10 +534,6 @@ def searchaddpl(context: MpdContext, *args: str) -> None:
     if not playlist:
         return  # TODO: Raise error about failed playlist creation?
 
-    try:
-        query = _query_for_search(parameters)
-    except ValueError:
-        return
     results = context.core.library.search(query).get()
     tracks = list(playlist.tracks) + _get_tracks(results)
     playlist = playlist.replace(tracks=tracks)
@@ -561,7 +562,7 @@ def update(context: MpdContext, uri: Uri | None = None) -> protocol.Result:
 
 # TODO: add at least reflection tests before adding NotImplemented version
 # @protocol.commands.add('readcomments')
-def readcomments(context: MpdContext, uri: Uri | None = None) -> None:
+def readcomments(context: MpdContext, uri: Uri) -> None:
     """
     *musicpd.org, music database section:*
 

src/mopidy_mpd/protocol/playback.py

@@ -232,12 +232,13 @@ def playid(context: MpdContext, tlid: int) -> None:
       replacement, starts playback at the first track.
     """
     if tlid == -1:
-        return _play_minus_one(context)
+        _play_minus_one(context)
+        return
 
     tl_tracks = context.core.tracklist.filter({"tlid": [tlid]}).get()
     if not tl_tracks:
         raise exceptions.MpdNoExistError("No such song")
-    return context.core.playback.play(tlid=tl_tracks[0].tlid).get()
+    context.core.playback.play(tlid=tl_tracks[0].tlid).get()
 
 
 @protocol.commands.add("previous")

src/mopidy_mpd/protocol/status.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Never
+from typing import TYPE_CHECKING
 
 from mopidy.core import PlaybackState
 
@@ -27,7 +27,7 @@
 
 
 @protocol.commands.add("clearerror")
-def clearerror(context: MpdContext) -> Never:
+def clearerror(context: MpdContext) -> protocol.Result:
     """
     *musicpd.org, status section:*
 

src/mopidy_mpd/translator.py

@@ -26,7 +26,8 @@ def track_to_mpd_format(  # noqa: C901, PLR0912
     """
     Format track for output to MPD client.
 
-    :param track: the track
+    :param obj: the track
+    :param tagtypes: the MPD tagtypes enabled by the client
     :param position: track's position in playlist
     :param stream_title: the current streams title
     """
@@ -120,7 +121,7 @@ def _has_value(
     Determine whether to add the tagtype to the output or not. The tagtype must
     be in the list of tagtypes configured for the client.
 
-    :param tagtypes: the MPD tagtypes configured for the client
+    :param tagtypes: the MPD tagtypes enabled by the client
     :param tagtype: the MPD tagtype
     :param value: the tag value
     """
@@ -183,8 +184,10 @@ def tracks_to_mpd_format(
     Optionally limit output to the slice ``[start:end]`` of the list.
 
     :param tracks: the tracks
+    :param tagtypes: the MPD tagtypes enabled by the client
     :param start: position of first track to include in output
-    :param end: position after last track to include in output
+    :param end: position after last track to include in output, or ``None`` for
+      end of list
     """
     if end is None:
         end = len(tracks)
@@ -210,6 +213,7 @@ def playlist_to_mpd_format(
     Format playlist for output to MPD client.
 
     :param playlist: the playlist
+    :param tagtypes: the MPD tagtypes enabled by the client
     :param start: position of first track to include in output
     :param end: position after last track to include in output
     """

tests/network/test_connection.py

@@ -326,10 +326,6 @@ def test_enable_timeout_does_not_add_timeout(self):
         network.Connection.enable_timeout(self.mock)
         assert GLib.timeout_add_seconds.call_count == 0
 
-        self.mock.timeout = None
-        network.Connection.enable_timeout(self.mock)
-        assert GLib.timeout_add_seconds.call_count == 0
-
     def test_enable_timeout_does_not_call_disable_for_invalid_timeout(self):
         self.mock.timeout = 0
         network.Connection.enable_timeout(self.mock)
@@ -339,10 +335,6 @@ def test_enable_timeout_does_not_call_disable_for_invalid_timeout(self):
         network.Connection.enable_timeout(self.mock)
         assert self.mock.disable_timeout.call_count == 0
 
-        self.mock.timeout = None
-        network.Connection.enable_timeout(self.mock)
-        assert self.mock.disable_timeout.call_count == 0
-
     @patch.object(GLib, "source_remove", new=Mock())
     def test_disable_timeout_deregisters(self):
         self.mock.timeout_id = sentinel.tag