5

I got a problem. Here is my bash script to merge videos with subs:

#!/bin/bash 

cd /media/ptrknvk/'TOSHIBA EXT'/Shows/My/'Doctor Who'/

url0='DW s11e0'
url1='DW\ s11e'

for(( i=1; i<11; i++ ))
do
    if ! [ $i -gt 9 ]; 
    then
        mkvmerge -o $url0$i.mkv $url0$i.mkv $url0$i.srt
        rm $url0$i.srt
    else
        mkvmerge -o $url1$i.mkv $url1$i.mkv $url1$i.srt
        rm $url1$i.srt
    fi
done

File have names like for example "DW s11e05.mkv" and they actually exist in the directory.

And here is a result:

mkvmerge v8.8.0 ('Wind at my back') 64bit
Error: The file 's11e09.mkv' could not be opened for reading: open file error.
rm: cannot remove 'DW': No such file or directory
rm: cannot remove 's11e09.srt': No such file or directory
mkvmerge v8.8.0 ('Wind at my back') 64bit
Error: The file 's11e10.mkv' could not be opened for reading: open file error.
rm: cannot remove 'DW\': No such file or directory
rm: cannot remove 's11e10.srt': No such file or directory

As you can see '\' before a space doesn't work here. I also tried to use echo, but nothing changed (mb I've used it in a wrong way).

Sorry for my little bit nooby style.

2
  • Yes, but there is cli question. It wasn't bash. Jan 2, 2019 at 14:37
  • bash is the default shell in Ubuntu. Whether in a script or in an interactive shell, quoting rules are exactly the same.
    – dessert
    Jan 2, 2019 at 15:57

2 Answers 2

11

You need to quote your variables. This should do what you want:

#!/bin/bash 

cd /media/ptrknvk/'TOSHIBA EXT'/Shows/My/'Doctor Who'/

url0='DW s11e0'
url1='DW s11e'

for(( i=1; i<11; i++ ))
do
    if ! [ $i -gt 9 ]; 
    then
        mkvmerge -o "$url0$i.mkv" "$url0$i.mkv" "$url0$i.srt" &&
        rm "$url0$i.srt"
    else
        mkvmerge -o "$url1$i.mkv" "$url1$i.mkv" "$url1$i.srt" &&
        rm "$url1$i.srt"
    fi
done

Note that I also added a && after each mkvmerge command so the rm only runs if the mkvmerge was successful. You don't want to delete the subtitle file if the merge failed!

That said, although there's absolutely nothing wrong with your approach, I feel you could make it considerable simpler and more elegant using shell globbing instead:

#!/bin/bash 

for file in "/media/ptrknvk/TOSHIBA EXT/Shows/My/Doctor Who/"DW*mkv; do
  srtFile=${file//.mkv/.srt}
  mkvmerge -o "$file" "$file" "$srtFile" && rm "$srtFile"
done

You don't even need a script for this. You can just run it directly in the terminal as a one liner:

for file in "/media/ptrknvk/TOSHIBA EXT/Shows/My/Doctor Who/"DW*mkv; do
  mkvmerge -o "$file" "$file" "${file//.mkv/.srt}" && rm "${file//.mkv/.srt}";
done
0
6

Quote variables to prevent word-splitting. By the way, double-quotes on a whole string is the preferred style.

#!/bin/bash

# Also exit if this fails
cd "/media/ptrknvk/TOSHIBA EXT/Shows/My/Doctor Who/" || exit

url0="DW s11e0"
url1="DW s11e"

for (( i=1; i<11; i++ )); do
    # Also why use "not greater-than" when "less-than-or-equal" exists?
    if [ $i -le 9 ]; then
        # Also you can DRY* out this part with variables.
        url="$url0"
    else
        url="$url1"
    fi
    # Also when concatenating variables, it's clearer to use the "${var}" style. **
    f_mkv="${url}${i}.mkv"
    f_srt="${url}${i}.srt"
    mkvmerge -o "$f_mkv" "$f_mkv" "$f_srt"
    rm "$f_srt"
done

Shellcheck is really useful for finding problems like this in shell scripts.

* DRY = Don't Repeat Yourself

** Or you could use printf -v, but it's not a big improvement in this case. For example

printf -v f_mkv "%s%s.mkv" "$url" $i
0

Not the answer you're looking for? Browse other questions tagged .