Ykhjuy

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.

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

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