Single instance of mpv while maintaining the coordinates of the previous window

Question

The code I am using is:

#!/bin/bash
pid=$(pidof "mpv")
if [[ "$pid" ]]; then
wmctrl -x -R gl.mpv
echo "{ \"command\": [\"loadfile\", \"1ドル\", \"append\"] }" | socat - "/tmp/mpv-socket"
echo playlist-next | socat - "/tmp/mpv-socket"
else
 mpv --input-ipc-server=/tmp/mpv-socket --player-operation-mode=pseudo-gui "1ドル"
fi

It makes sure that there is only one instance of mpv. It also make sure that the new instance is open in the previous window's position. It also has memory. I mean, it adds to the playlist then play it. So, I can go to the previous video.

Please note that socat must be installed in the machine.

How can I make this code better?

Answer 1

Make the important elements easy to see

In the posted code I mean the script parameter 1ドル. Currently it appears in the middle of the code in two places. Until I take a closer look, I don't even know the script takes a parameter.

I suggest to store the script parameters early in the script in a variable. And make sure the variable has a descriptive name, it's crucial to help readers understand the meaning and purpose. In this example it seems the parameter is a path to a file, so I suggest to do this near the top of the script:

path=1ドル

gl.mpv might be a good candidate too, to give a descriptive name near the top of the script.

Validate parameters

In both branches of the if-statement, the parameter is used. Therefore it seems it's a required parameter for the script to work well. Here's one way to validate this and exit when the parameter is missing:

if [[ $# != 1 ]]; then
 echo >&2 "Missing parameter: path-to-file"
 echo >&2 "usage: 0ドル path-to-file"
 exit 1
fi

And if the parameter must be a path to a file, it probably makes sense to also verify that:

if ! [[ -f 1ドル ]]; then
 echo >&2 "Not a file: 1ドル"
 exit 1
fi

Let Bash raise errors on suspicious operations

I start all my Bash scripts with this, to mitigate the impact of certain mistakes:

set -euo pipefail

This does a bunch of helpful things:

• -e makes the script abort on the first failing command, which can help in many ways
  • avoid disasters by failing fast
  • highlight that something unexpected happened
  • make the failure easy to see, at the end of the script output
  • encourages implementing solid error handling
• -u makes it an error when referencing an undefined variable; together with -e, this again can help avoid disasters
• -o pipefail makes the exit code of a pipeline success only when all commands exit with success (you can read more about it in man bash, search for "pipefail")

Don't repeat yourself

socat - "/tmp/mpv-socket" appears twice in the script. You can avoid duplicated logic using a function:

socat_to_mpv() {
 socat - "/tmp/mpv-socket"
}

And then use ... | socat_to_mpv in the pipelines.

Improve readability using single quotes

This expression is a bit hard to read because of all the escaping of embedded double-quotes:

echo "{ \"command\": [\"loadfile\", \"1ドル\", \"append\"] }"

In situations like this you could use single quotes instead, but carefully:

echo '{ "command": ["loadfile", "'"1ドル"'", "append"] }'

Notice what's happening around the 1ドル there. The basic pattern here is this:

echo '...'"1ドル"'...'

That is, when you want to embed a variable in a single-quoted string, "interrupt" the single-quoted string with a ', double-quote the variable as recommended for safety, and then start again the single-quoted string for the remaining part.

Indent the body of if-else statements

In the posted code the if-branch is not indented, which makes it a bit harder to read. That is, instead of this:

if [[ "$pid" ]]; then
...
else
 ...
fi

Better to write like this:

if [[ "$pid" ]]; then
 ...
else
 ...
fi