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:
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.
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.
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.
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.
else' ./script.sh: line 26:
else' " – xqj695 Apr 24 '18 at 01:50read destFile
butcp
to$destinationFile
; you don't assignsourceFile
at all); yourwhile
is missing itsdo
anddone
;[ $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. – steeldriver Apr 24 '18 at 01:54