6
\$\begingroup\$

About

This project attempts to rename a collection of .mp4 files using a wordlist from two different folders. The script reads filenames from a text file, gathers video files from a target directory, sorts it out and then renames each file sequentially based on the wordlist order.

#!/bin/bash

cd -- /home/TonySoprano /File_Rename_Project

   IFS=$'\n' read -r -d '' -a wordlist < <( cat  Prepped.txt && printf '\0')

            cd -- /mnt/c/Users/gabagool /dwhelper/"Courses"
                echo  "Renaming files in "$PWD""

                shopt -s nocaseglob
                myFiles=(*.mp4)
                
IFS=$'\n' read -r -d '' -a my_array < <(printf "%s\n" "${myFiles[@]}"| sort -V  && printf '\0')
        
                  count=0 

                for i in "${my_array[@]}";
                        do 

                          mv -v "$i" "${wordlist[$count]}"
                                        ((count++))
                done

Current Workflow + Structure

Changes into a directory to read a wordlist file.

  • It loads the contents of that file into an array, to prevent word splits.

  • It then changes into a second directory containing video files.

  • Enables case insensitive globbing, collects all .mp4 files, sorts them using version.

  • Takes each mp4 file and is replaced with chosen wordlist.

The program is messy and here are my weak spots I’ve found to improve on

  • pathfinding (cd):

    cd is repeated twice so it feels like the main cause of the messy looking program.

  • shopt -s nocaseglob; myfiles=(*.mp4):
    Were placed inside the code to math .mp4/.MP4 and created myfiles variable to make easier to index.

Is there anything that I have not considered?

Context:
I was getting frustrated from file naming on Windows and got tired of right clicking. This prompted me to pick up programming, but I did not expect this to cost me three weeks of my health and sleepless nights just to shave off some time. It was worth it in end.

New contributor
Thejuan andOnly is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
2
  • 3
    \$\begingroup\$ Why the odd indentation? \$\endgroup\$ Commented 15 hours ago
  • \$\begingroup\$ What is cd -- /home/TonySoprano /File_Rename_Project (with two arguments) supposed to do? Or is that a typo and should be cd -- /home/TonySoprano/File_Rename_Project ? \$\endgroup\$ Commented 4 hours ago

4 Answers 4

7
\$\begingroup\$

BaDaBing


cd -- /home/TonySoprano /File_Rename_Project
...
    cd -- /mnt/c/Users/gabagool /dwhelper/"Courses"

Ba Da Boom!

chdir takes a single argument.

You can try to give it more. But it won't do you any good.

indent

           cd -- ...
               echo ...

I have no idea why those are at different indent levels. Please use the same indent level for commands in the same block. And try to stick to a multiple of 4 spaces.

Indenting the body of a for loop makes sense, but the rest of what you've written is quite hard to follow along with. You've not made it easy to recruit maintenance engineers to your team who will be able to effectively understand your code and make changes to it.

\$\endgroup\$
5
\$\begingroup\$

Error checking

When we use cd, we don't check whether it was successful. That may mean continuing to operate in the wrong directory. We should abort execution if it fails - that may be as simple as cd $dir || exit.

There are other commands in the script whose failure should cause early exit or be reflected in the script's own exit status. Don't assume that anything will be successful!

\$\endgroup\$
4
\$\begingroup\$

Layout

As mentioned in the @J_H answer, the indentation is inconsistent. Here is a more common way to indent the code:

#!/bin/bash

cd -- /home/TonySoprano /File_Rename_Project

IFS=$'\n' read -r -d '' -a wordlist < <( cat  Prepped.txt && printf '\0')

cd -- /mnt/c/Users/gabagool /dwhelper/"Courses"
echo  "Renaming files in "$PWD""

shopt -s nocaseglob
myFiles=(*.mp4)

IFS=$'\n' read -r -d '' -a my_array < <(printf "%s\n" "${myFiles[@]}"| sort -V  && printf '\0')

count=0 

for i in "${my_array[@]}";
do 
    mv -v "$i" "${wordlist[$count]}"
    ((count++))
done

I also removed the blank line inside the for loop since there is no real need for it.

Portability

When I try to run the code in my Linux OS, I see this kind of message for each cd line:

cd: too many arguments

Perhaps you also get those messages in your OS, but you might have missed them if they were mixed in with your other output.

Tools

The ShellCheck tool can be used to analyze your code for potential bugs and other style issues. When I run it, it is pretty clean, with only a few minor complaints. Good job there.

Naming

You used a few different naming styles for your variables. You should pick a style and stick to it. I prefer snake_case, where words are all lower-case and separated by an underscore:

word_list
my_files

You could give the variables more meaningful names as well. Using words like "my", "list" and "array" are too generic. For example, "wordlist" could be "file_names, "myFiles" could be "video_files", and "my_array" could be "sorted_files".

While i is a common iterator variable name, it is typically used for numbers. However, in your code it represents a file name. You could use file_name instead of i.

\$\endgroup\$
4
\$\begingroup\$

Just one thing I don't think others have pointed out yet:

printf "%s\n" "${myFiles[@]}"| sort -V

requires that your file names not contain newlines or you'd be sorting parts of file names so then there's no need to add surrounding code like this:

IFS=$'\n' read -r -d '' -a my_array < <(printf "%s\n" "${myFiles[@]}"| sort -V  && printf '\0')

when you could just use:

readarray -t my_array < <(printf "%s\n" "${myFiles[@]}"| sort -V)

I also strongly suspect that this:

IFS=$'\n' read -r -d '' -a wordlist < <( cat  Prepped.txt && printf '\0')

could similarly just be:

readarray -t wordlist < Prepped.txt

Btw, you don't actually need the myFiles array, you could just do printf '%s\n' *.mp4

\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.