From 72715845a10dfabaee364f70af7c33b26b2d1c0d Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Tue, 20 Jun 2023 14:55:25 -0400
Subject: [PATCH] display destination file before youtube-dl download

Rather than after it, which can leave one wondering what file it's
downloading.

youtubeDl should not ever return Right Nothing in normal operation,
becaause it's already asked youtube-dl if it supports the url. So it
would have to succeed at that, then not download any file, but also
exit successfully, in order for the new error message to display.

Also display the name of yt-dlp when using it.
---
 Annex/YoutubeDl.hs | 18 +++++++++++-------
 Command/AddUrl.hs  | 12 ++++++++----
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/Annex/YoutubeDl.hs b/Annex/YoutubeDl.hs
index 5ab4d2bc06..0af0637061 100644
--- a/Annex/YoutubeDl.hs
+++ b/Annex/YoutubeDl.hs
@@ -12,6 +12,7 @@ module Annex.YoutubeDl (
 	youtubeDlCheck,
 	youtubeDlFileName,
 	youtubeDlFileNameHtmlOnly,
+	youtubeDlCommand,
 ) where
 
 import Annex.Common
@@ -74,18 +75,18 @@ youtubeDl' url workdir p uo
 			( runcmd cmd >>= \case
 				Right True -> downloadedfiles cmd >>= \case
 					(f:[]) -> return (Right (Just f))
-					[] -> return nofiles
-					fs -> return (toomanyfiles fs)
+					[] -> return (nofiles cmd)
+					fs -> return (toomanyfiles cmd fs)
 				Right False -> workdirfiles >>= \case
 					[] -> return (Right Nothing)
-					_ -> return (Left "yt-dlp download is incomplete. Run the command again to resume.")
+					_ -> return (Left $ cmd ++ " download is incomplete. Run the command again to resume.")
 				Left msg -> return (Left msg)
 			, return (Right Nothing)
 			)
 	| otherwise = return (Right Nothing)
   where
-	nofiles = Left "yt-dlp did not put any media in its work directory, perhaps it's been configured to store files somewhere else?"
-	toomanyfiles fs = Left $ "yt-dlp downloaded multiple media files; git-annex is only able to deal with one per url: " ++ show fs
+	nofiles cmd = Left $ cmd ++ " did not put any media in its work directory, perhaps it's been configured to store files somewhere else?"
+	toomanyfiles cmd fs = Left $ cmd ++ " downloaded multiple media files; git-annex is only able to deal with one per url: " ++ show fs
 	downloadedfiles cmd
 		| isytdlp cmd = liftIO $ 
 			(lines <$> readFile filelistfile)
@@ -95,7 +96,7 @@ youtubeDl' url workdir p uo
 		<$> (filterM (doesFileExist) =<< dirContents workdir)
 	filelistfile = workdir </> filelistfilebase
 	filelistfilebase = "git-annex-file-list-file"
-	isytdlp cmd = "yt-dlp" `isInfixOf` cmd
+	isytdlp cmd = cmd == "yt-dlp"
 	runcmd cmd = youtubeDlMaxSize workdir >>= \case
 		Left msg -> return (Left msg)
 		Right maxsize -> do
@@ -271,7 +272,10 @@ youtubeDlOpts addopts = do
 youtubeDlCommand :: Annex String
 youtubeDlCommand = annexYoutubeDlCommand <$> Annex.getGitConfig >>= \case
 	Just c -> pure c
-	Nothing -> fromMaybe "youtube-dl" <$> liftIO (searchPath "yt-dlp")
+	Nothing -> ifM (liftIO $ inSearchPath "yt-dlp")
+		( return "yt-dlp"
+		, return "youtube-dl"
+		)
 
 supportedScheme :: UrlOptions -> URLString -> Bool
 supportedScheme uo url = case parseURIRelaxed url of
diff --git a/Command/AddUrl.hs b/Command/AddUrl.hs
index 988131c89e..018dedc882 100644
--- a/Command/AddUrl.hs
+++ b/Command/AddUrl.hs
@@ -353,21 +353,25 @@ downloadWeb addunlockedmatcher o url urlinfo file =
 	  where
 		dl dest = withTmpWorkDir mediakey $ \workdir -> do
 			let cleanuptmp = pruneTmpWorkDirBefore tmp (liftIO . removeWhenExistsWith R.removeLink)
-			showNote "using youtube-dl"
+			dlcmd <- youtubeDlCommand
+			showNote ("using " <> UnquotedString dlcmd)
 			Transfer.notifyTransfer Transfer.Download url $
-				Transfer.download' webUUID mediakey (AssociatedFile Nothing) Nothing Transfer.noRetry $ \p ->
+				Transfer.download' webUUID mediakey (AssociatedFile Nothing) Nothing Transfer.noRetry $ \p -> do
+					showDestinationFile dest
 					youtubeDl url (fromRawFilePath workdir) p >>= \case
 						Right (Just mediafile) -> do
 							cleanuptmp
 							checkCanAdd o dest $ \canadd -> do
-								showDestinationFile dest
 								addWorkTree canadd addunlockedmatcher webUUID mediaurl dest mediakey (Just (toRawFilePath mediafile))
 								return $ Just mediakey
-						Right Nothing -> checkRaw Nothing o (pure Nothing) (normalfinish tmp backend)
 						Left msg -> do
 							cleanuptmp
 							warning (UnquotedString msg)
 							return Nothing
+						Right Nothing -> do
+							cleanuptmp
+							warning (UnquotedString dlcmd <> " did not download anything")
+							return Nothing
 		mediaurl = setDownloader url YoutubeDownloader
 		mediakey = Backend.URL.fromUrl mediaurl Nothing
 		-- Does the already annexed file have the mediaurl