-2

I'm trying to write a bash script to prompt a user for a filename, and if the file exists, allow them to overwrite or append the file. For some reason, I keep getting returned with a syntax error despite having an issue. Unfortunately, I am not as familiar with Bash as I am with Python, and find myself confusing the syntax of the two. Nevertheless, please see below:

#!/bin/bash
echo "Please enter the name of your destination file: "
read destFile
if [ -f "$destFile" ]; then
    echo "This file exists."
fi
echo "Please enter the name of your source file: "
read sourceFile
echo "Do you want to 1) Overwrite, 2) Append, 3) Exit?"
echo "Please select your option: "
options=("Overwrite" "Append" "Exit")
while [ -e "$sourceFile" ]; do
    select opt in "${options[@]}"; do
        case "$opt" in
        "Overwrite")
            cp "$sourceFile" "$destFile"
            echo "Copy Completed."  
            ;;
        "Append" )
            cat "$sourceFile" >> "$destFile"
            echo "Append Completed."
            ;;
        "Exit" )
            echo "Script Terminated"
            break
            ;;
        esac
        break
    done
done
8
  • 1
    Please post the syntax error
    – wjandrea
    Apr 24, 2018 at 1:43
  • "./script.sh: line 26: syntax error near unexpected token else' ./script.sh: line 26: else' "
    – xqj695
    Apr 24, 2018 at 1:50
  • 2
    Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
    – wjandrea
    Apr 24, 2018 at 1:51
  • 1
    There are really too many errors to list - some obvious typos (e.g. you read destFile but cp to $destinationFile; you don't assign sourceFile at all); your while is missing its do and done; [ $sourcefile -e ] should be [ -e "$sourceFile" ] (in fact you should quote ALL the variable expansions); no shebang to indicate what interpreter to use. I suggest you start with www.shellcheck.net and post back if you are still unable to resolve. Apr 24, 2018 at 1:54
  • 2
    @xqj695 Please let me know if my answer helps, and if there's anything you're unsure about or think I may have made a mistake on or misunderstood what you're trying to do. I had to guess slightly about your script's intended behavior; I went with what you wrote at the top (as I understood it). Whether or not you find my answer helpful, I very much recommend you edit again to document all steps you took, what you learned in each step, and, most important, your questions as they apply to the script you now show. I had to rely on comments to write my answer, which is far from ideal. Apr 24, 2018 at 3:47

1 Answer 1

4

As you stated it, the problem you're trying to solve is (emphasis added):

I'm trying to write a bash script to prompt a user for a filename, and if the file exists, allow them to overwrite or append the file.

Curiously, the code in your script that checks if the file exists does not use if, though it probably should. Instead, it uses while:

while [ -e "$sourceFile" ]; do

That's a loop. If the source file exists, everything inside that loop, up to the matching

done

on the last line of your script runs. This is a while loop, so after it runs the first time test is performed again. If the file still exists, the body of the loop runs again, and so on, and so on, and so on.

But nothing in the body of the loop ever causes the file to stop existing. The four possibilities are:

  1. The user enters a 1 and the source file is copied over the destination file. It is copied, not moved, so the source file still exists, with the same name.

  2. The user enters a 2 and the contents of the source file are appended to the destination file. However, the source file is not deleted; it's still there.

  3. The user enters a 3 and a Script Terminated message is printed. This is followed immediately by a break command. This breaks out of the select construct. The summary of break that you get when you run help break only mentions that it breaks out of for, while, or until loops, so perhaps you expected it to break the outer while loop. However, the summary of select you get when you run help select clarifies the matter:

    COMMANDS are executed after each selection until a break command is executed.

  4. The user enters anything other than 1, 2, or 3. So none of the cases runs. The second break command, after esac, runs. This, too, breaks the select construct, not the while loop, and so the while loop runs again.

select is actually a loop. Your break commands break the select rather than the outer while. Given the problem description you provided, you almost certainly don't have any reason to use an outer loop. So one good solution would be for you to change the outer while into an if. To do that, you will also need to:

  • Change its do into a then.
  • Change its done into a fi.

If you choose this solution, then you will also want to move some of the commands that you currently have outside the while (which should be an if) so that they are inside it. Right now, the user is prompted to select an option even if no action would actually be taken. That additional bug would not be fixed merely changing the while to an if (and making the other two required changes for that to work).

An alternative solution is to make your script exit early if [ -e "$sourceFile" ] is false. One way to do that is:

[ -e "$sourceFile" ] || exit

Another, if you prefer to use if, is:

if ! [ -e "$sourceFile" ]; then
    exit
fi

If you use either of those, then the rest of the code -- the code that you intend to run only if the source file exists -- would not need to be enclosed in any control structure. Note that you will still want that to come before, rather than after, the commands that prompt the user for what action to take.

Note that you could "fix" your code by replacing your break commands with exit. You could also "fix" it by passing a numeric argument to break to tell it how many levels of nesting to break out of (most languages' break commands don't support that, but Bash's does). However, I recommend against using either of those approaches -- except perhaps to just to try it out -- because you would be adding more complexity to code that should instead be fixed by making it simpler. Currently you are using a while loop for something where no loop makes sense, so however you solve the problem, it should be in a way that lets you simplify the code by not doing that.

If, on the other hand, you really do intend the whole thing to run over and over again, then you should consider what condition would meaningfully break your outer loop. For example, perhaps you meant to prompt for source and destination files in each iteration of the outer loop. If so, then you'll need to change your code to do that.

Finally, I recommend indenting your code in such a way that, when a control structure spans multiple lines, the line that opens it and the line that closes it are indented at the same level and the lines inside it (except in occasional situations where it is infeasible to do so) are indented more. That way, you will know which fi matches which if, which done matches which while/until/for/select, and which esac matches which case. By being able to recognize that more easily, you'll also be able to better identify when a needed keyword is missing, and by indenting the contents in a consistent way, you'll be able to better identify when the logic if your script is different from what you intend.

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