Bash script to overwrite/append destination file with options menu [duplicate]


up vote
-2
down vote
favorite
This question already has an answer here:
How to debug bash script?
7 answers
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
bash scripts copy
marked as duplicate by Eric Carvalho, Fabby, waltinator, vidarlo, George Udosen May 6 at 20:29
This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.
 |Â
show 3 more comments
up vote
-2
down vote
favorite
This question already has an answer here:
How to debug bash script?
7 answers
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
bash scripts copy
marked as duplicate by Eric Carvalho, Fabby, waltinator, vidarlo, George Udosen May 6 at 20:29
This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.
1
Please post the syntax error
â wjandrea
Apr 24 at 1:43
"./script.sh: line 26: syntax error near unexpected tokenelse' ./script.sh: line 26:
else' "
â xqj695
Apr 24 at 1:50
2
Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
â wjandrea
Apr 24 at 1:51
1
There are really too many errors to list - some obvious typos (e.g. youread 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 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.
â Eliah Kagan
Apr 24 at 3:47
 |Â
show 3 more comments
up vote
-2
down vote
favorite
up vote
-2
down vote
favorite
This question already has an answer here:
How to debug bash script?
7 answers
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
bash scripts copy
This question already has an answer here:
How to debug bash script?
7 answers
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
This question already has an answer here:
How to debug bash script?
7 answers
bash scripts copy
edited Apr 24 at 3:33


wjandrea
7,15342255
7,15342255
asked Apr 24 at 1:00
xqj695
61
61
marked as duplicate by Eric Carvalho, Fabby, waltinator, vidarlo, George Udosen May 6 at 20:29
This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.
marked as duplicate by Eric Carvalho, Fabby, waltinator, vidarlo, George Udosen May 6 at 20:29
This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.
1
Please post the syntax error
â wjandrea
Apr 24 at 1:43
"./script.sh: line 26: syntax error near unexpected tokenelse' ./script.sh: line 26:
else' "
â xqj695
Apr 24 at 1:50
2
Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
â wjandrea
Apr 24 at 1:51
1
There are really too many errors to list - some obvious typos (e.g. youread 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 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.
â Eliah Kagan
Apr 24 at 3:47
 |Â
show 3 more comments
1
Please post the syntax error
â wjandrea
Apr 24 at 1:43
"./script.sh: line 26: syntax error near unexpected tokenelse' ./script.sh: line 26:
else' "
â xqj695
Apr 24 at 1:50
2
Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
â wjandrea
Apr 24 at 1:51
1
There are really too many errors to list - some obvious typos (e.g. youread 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 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.
â Eliah Kagan
Apr 24 at 3:47
1
1
Please post the syntax error
â wjandrea
Apr 24 at 1:43
Please post the syntax error
â wjandrea
Apr 24 at 1:43
"./script.sh: line 26: syntax error near unexpected token
else' ./script.sh: line 26:
else' "â xqj695
Apr 24 at 1:50
"./script.sh: line 26: syntax error near unexpected token
else' ./script.sh: line 26:
else' "â xqj695
Apr 24 at 1:50
2
2
Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
â wjandrea
Apr 24 at 1:51
Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
â wjandrea
Apr 24 at 1:51
1
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.â steeldriver
Apr 24 at 1:54
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.â steeldriver
Apr 24 at 1:54
2
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.
â Eliah Kagan
Apr 24 at 3:47
@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.
â Eliah Kagan
Apr 24 at 3:47
 |Â
show 3 more comments
1 Answer
1
active
oldest
votes
up vote
4
down vote
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 aScript Terminated
message is printed. This is followed immediately by abreak
command. This breaks out of theselect
construct. The summary ofbreak
that you get when you runhelp break
only mentions that it breaks out offor
,while
, oruntil
loops, so perhaps you expected it to break the outerwhile
loop. However, the summary ofselect
you get when you runhelp select
clarifies the matter:
COMMANDS are executed after each selection until a break command is executed.
The user enters anything other than
1
,2
, or3
. So none of the cases runs. The secondbreak
command, afteresac
, runs. This, too, breaks theselect
construct, not thewhile
loop, and so thewhile
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 athen
. - Change its
done
into afi
.
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.
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
4
down vote
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 aScript Terminated
message is printed. This is followed immediately by abreak
command. This breaks out of theselect
construct. The summary ofbreak
that you get when you runhelp break
only mentions that it breaks out offor
,while
, oruntil
loops, so perhaps you expected it to break the outerwhile
loop. However, the summary ofselect
you get when you runhelp select
clarifies the matter:
COMMANDS are executed after each selection until a break command is executed.
The user enters anything other than
1
,2
, or3
. So none of the cases runs. The secondbreak
command, afteresac
, runs. This, too, breaks theselect
construct, not thewhile
loop, and so thewhile
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 athen
. - Change its
done
into afi
.
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.
add a comment |Â
up vote
4
down vote
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 aScript Terminated
message is printed. This is followed immediately by abreak
command. This breaks out of theselect
construct. The summary ofbreak
that you get when you runhelp break
only mentions that it breaks out offor
,while
, oruntil
loops, so perhaps you expected it to break the outerwhile
loop. However, the summary ofselect
you get when you runhelp select
clarifies the matter:
COMMANDS are executed after each selection until a break command is executed.
The user enters anything other than
1
,2
, or3
. So none of the cases runs. The secondbreak
command, afteresac
, runs. This, too, breaks theselect
construct, not thewhile
loop, and so thewhile
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 athen
. - Change its
done
into afi
.
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.
add a comment |Â
up vote
4
down vote
up vote
4
down vote
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 aScript Terminated
message is printed. This is followed immediately by abreak
command. This breaks out of theselect
construct. The summary ofbreak
that you get when you runhelp break
only mentions that it breaks out offor
,while
, oruntil
loops, so perhaps you expected it to break the outerwhile
loop. However, the summary ofselect
you get when you runhelp select
clarifies the matter:
COMMANDS are executed after each selection until a break command is executed.
The user enters anything other than
1
,2
, or3
. So none of the cases runs. The secondbreak
command, afteresac
, runs. This, too, breaks theselect
construct, not thewhile
loop, and so thewhile
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 athen
. - Change its
done
into afi
.
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.
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 aScript Terminated
message is printed. This is followed immediately by abreak
command. This breaks out of theselect
construct. The summary ofbreak
that you get when you runhelp break
only mentions that it breaks out offor
,while
, oruntil
loops, so perhaps you expected it to break the outerwhile
loop. However, the summary ofselect
you get when you runhelp select
clarifies the matter:
COMMANDS are executed after each selection until a break command is executed.
The user enters anything other than
1
,2
, or3
. So none of the cases runs. The secondbreak
command, afteresac
, runs. This, too, breaks theselect
construct, not thewhile
loop, and so thewhile
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 athen
. - Change its
done
into afi
.
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.
edited Apr 24 at 3:51
answered Apr 24 at 3:41
Eliah Kagan
79.4k20221359
79.4k20221359
add a comment |Â
add a comment |Â
1
Please post the syntax error
â wjandrea
Apr 24 at 1:43
"./script.sh: line 26: syntax error near unexpected token
else' ./script.sh: line 26:
else' "â xqj695
Apr 24 at 1:50
2
Please edit it into the question. Also, please fix the indenting. That makes it easier to read.
â wjandrea
Apr 24 at 1:51
1
There are really too many errors to list - some obvious typos (e.g. you
read 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 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.
â Eliah Kagan
Apr 24 at 3:47