patternbashMinor
Notification script | from RSS to Email | Bash
Viewed 0 times
scriptrssemailfrombashnotification
Problem
This is a script that must send an email at each new article published on a specific website. Any suggestions or improvements to do?
SENDER_EMAIL="sender@example.com"
TO_EMAIL="myemail@example.com"
RSS_SITE="example.com/feed.xml"
CHECK_INTERVAL=10
while [ 1 ]; do
LINK_ARTICLE=$(rsstail -i 1 -u $RSS_SITE -l -n 0 -1 | grep -oP "Link:+ \K.*")
TITLE_ARTICLE=$(rsstail -i 1 -u $RSS_SITE -n 0 -1 | grep -oP "Title:+ \K.*")
if [ "$LINK_ARTICLE" != "" ] && [ "$TITLE_ARTICLE" != "" ]; then
echo "New article published on the site. TITLE: $TITLE_ARTICLE - LINK: $LINK_ARTICLE" | EMAIL="$SENDER_EMAIL" mutt -s "Nuovo Articolo BDO" "$TO_EMAIL"
echo "New article published on the site. TITLE: $TITLE_ARTICLE - LINK: $LINK_ARTICLE"
fi
sleep $CHECK_INTERVAL
doneSolution
I see a number of things that may help you improve your code.
Use "shebang" line
The shebang is the line at the beginning of a shell script that tells which program to use. In this case, you probably want this:
See this question for details.
Consider using
If this is something you want to run automatically, consider running it as a cron tab instead of using
Include some comments
The program requires
Be cautious about handing variables to programs
The
Combine strings
The
Avoid creating extraneous variables
Instead of creating
Consider writing a portable script
By sticking closely with Posix and avoiding bashisms your code could run on many different kinds of systems, including recent versions of Ubuntu which don't use
Use "shebang" line
The shebang is the line at the beginning of a shell script that tells which program to use. In this case, you probably want this:
#! /usr/bin/env bashSee this question for details.
Consider using
cron instead of sleepIf this is something you want to run automatically, consider running it as a cron tab instead of using
sleep within the script.Include some comments
The program requires
rsstail, mutt and sleep which is a requirement that should be documented in a comment.Be cautious about handing variables to programs
The
mutt program, like many Linux programs, has a -- option which specifies that no further options are on the command line. This prevents the contents of $TO_EMAIL in a line like the following from being misinterpreted as a command line option.mutt -s $TITLE -- $TO_EMAIL < $BODYTEXTCombine strings
The
echo is used twice with an identical string. An alternative approach is TITLE="Nuovo Articolo BDO"
BODYTEXT="New article published on the site. TITLE: $TITLE_ARTICLE - LINK: $LINK_ARTICLE"
mutt -s $TITLE -- $TO_EMAIL < $BODYTEXT
echo $BODYTEXTAvoid creating extraneous variables
Instead of creating
SENDER_EMAIL, you could just specify EMAIL and then the reassignment of the latter variable before mutt is called would not be necessary.Consider writing a portable script
By sticking closely with Posix and avoiding bashisms your code could run on many different kinds of systems, including recent versions of Ubuntu which don't use
bash.Code Snippets
#! /usr/bin/env bashmutt -s $TITLE -- $TO_EMAIL < $BODYTEXTTITLE="Nuovo Articolo BDO"
BODYTEXT="New article published on the site. TITLE: $TITLE_ARTICLE - LINK: $LINK_ARTICLE"
mutt -s $TITLE -- $TO_EMAIL < $BODYTEXT
echo $BODYTEXTContext
StackExchange Code Review Q#114536, answer score: 7
Revisions (0)
No revisions yet.