cli: log resolved file output path #4336
Conversation
|
It's very clear to me even with the spaces since it is on a separate line I'd say this is fine. Can leave this open for a bit if you want feedback from others. |
|
I think with the full path output, the path in the confirm message could be removed and that would be reasonable and still obvious, although I also think it's fine as it is. To me also, it is clear as it is. Personally, I would prefer not fully escaped, as that could be annoying if you wanted to copy and paste the full path/file for other reasons. At most, it could be wrapped in single or double quotes. Single quotes offer the most protection on UNIX-like systems of course, but I seem to remember an issue on Windows systems with single quotes where they don't get consumed by the shell before getting passed on to an application. That of course could be made system-specific if necessary and you wanted to go down the quoting route... Overall, what you have now looks perfectly good to me, IMHO. |
Streamlink currently doesn't log the full path when writing the output to a file. This is an issue when the output path is dynamically generated from the input argument via the stream's metadata, because the user doesn't know where the data was actually written to. The output path gets already logged if the file already exists, but that's not really helpful.
The log message added by this PR does only get printed if the stream is written to a file, and not to stdout via
--stdoutor--output=-.I'm not sure though if we should put the path into quotes and escape it (via shlex.quote), because if there are spaces, it's not perfectly clear. It is however on a separate line in the output, so maybe it is. The file path in the confirm message could also be removed, but that would require the "Writing output to" message to be printed via
console.msgand notlog.info, because log messages can be suppressed whereas console messages can not.Thoughts?