[SOLVED] over my head in bash

David the H. · 08-17-2011, 10:21 PM

Good to see you got it working. That's a nice start on the script.

A handful of suggestions...

$(..) is highly recommended over `..`. I cringe every time I see backticks. I also suggest using [[ for all your tests, as I assume the script won't need to be portable to non-bash systems.

Get into the habit of quoting all your parameter expansions, unless you need word splitting to occur. This includes both variables and command substitutions. It's safer that way.

The pattern "day=()" initializes the variable as an array. Not a problem, but unnecessary.

If a variable isn't needed globally, then you should use local (or declare) to define it as local to a function. You're "havdalah" variable is one possible example. You should probably also not use the same name for both a variable and a function, to avoid confusion.

I'd also use indentation a bit more, for clarity, particularly in functions and loops. This is a personal style preference, however. You also don't need to end every command with a semicolon. This isn't perl.

Finally, your day_of_week function has an unnecessary double-running of date. It would be better structured this way:

Code:

function day_of_week 	#set the day of the week
{
     Day="$( date +%a )"
     if [[ "$Day" != "Fri" ]]; then
          Day="$( date -d "next fri" +%a )"
     fi
}

( Edit: Come to think of it, what's the purpose of this function anyway? The output is always going to be "Day=Fri". )

Similarly, your set_date function can be simplified to this:

Code:

function set_date
{
  # Get the current date and set the variables day, month, year.
    read day month year <<<"$( date +"%d %m %Y" -d "next $Day" )"
}

grail · 08-18-2011, 12:02 AM

+1 to David's suggestion on indentation. I would also add that having expressions of tests on a line (assuming not too long)
and then the effect of the expression on its own line makes things clearer.

Example:

Code:

# Your current code
if [ `date +%a` == "Fri" ];
  then Day=`date +%a`;
  else Day=`date -d "next fri" +%a` ;
fi

# my suggestion (same as David's example above but keeping your format to show the difference)
if [ `date +%a` == "Fri" ]; then
    Day=`date +%a`
else
    Day=`date -d "next fri" +%a`
fi

This is not to suggest David's rewrite is lacking (in fact I prefer it), this is simply to illustrate the difference

rbees · 08-18-2011, 07:44 AM

grrrr

I had a response all written up and when I tried to post it I lost it

grrr

's what I get for using a browser for an editor

Anyway, Thanks

I don't have time right now to go through it again in very much detail but the day_of_week function is so that the script will return the results for the up coming Shabbat no matter what day of the week the script is run.

Will post again when I get the script rewritten per your instructions.

Thanks

David the H. · 08-18-2011, 05:56 PM

Quote:

Originally Posted by rbees

I had a response all written up and when I tried to post it I lost it

There are extensions out there that will auto-save your text. I use Textarea Cache for Firefox myself.

https://addons.mozilla.org/en-US/fir...extarea-cache/

Quote:

...the day_of_week function is so that the script will return the results for the up coming Shabbat no matter what day of the week the script is run.

I get what you're trying to do, but with the current logic of the function, you're never going to get anything but "Fri" for output (it comes out to "if today isn't Friday, make it Friday"). You might as well replace it with a simple "Day=Fri" variable, unless you plan on adding something else to it as well, like a warning message or such.

rbees · 08-19-2011, 07:36 AM

I was under the impression that back tic's were required with commands from the reading I have done.

Code:

Get into the habit of quoting all your parameter expansions

".." or '..' Somewhere I read something about when to use each and there was a difference but I can find it now. oot, found it http://www.panix.com/~elflord/unix/bash-tute.html

well it seams that hdate does not like "$var". The " cause hdate to interpret the variable incorrectly somehow.

I was not able to get " or ( to work around the $lat or $long variables.

Quote:

"havdalah" variable is one possible example. You should probably also not use the same name for both a variable and a function, to avoid confusion.

I knew that variables and functions should have different names, missed that I was doing it. Havdalah actually means separation and the Yom Tov (holidays/festivals) also have a havdalah time associated with them just as they do a candle lighting time.

I was also under the impression that I needed to end every line with a ; and originally had quite some trouble getting things to work until I started putting the ; at the end of my lines and so stayed with it. Will continuing the practice aid in converting the script to a complied program at some point? If so, and there is not a good reason to discontinue the practice maybe I will stay with it.

What I lost in the glitch (an issue with my wireless access point) was an explanation of how hdate interacts with Shabbat and the Gregorian calendar our computers use. A single Hebrew day spans two Gregorian days because the day starts at sundown and not some arbitrary time after dark. So to get candle lighting time for the start of Shabbat requires that the day of the week be Friday but to get the Torah (books of Moses) reading, and several other things the day of the week needs to be set Saturday.

I have my code rewritten now;

Code:

#!/bin/bash 


# Script to play a Shofar blast for the start of Shabbat among other things

##### Constants

lat=44   	# l variable in hdate
long=-85	# L variable in hdate
tz="$( date +%:::z )"
day=()
month=()
year=()
Day=()
havdalah=()

##### Functions

function Long
{
  # At some point we will make this interactive or set through a config file.  -L
  -85
}

function Lat
{
  # At some point we will make this interactive or set through a config file.  -l
  44
}

function Tz
{
  # At some point we will make this interactive or set through a config file.
  -5
}

function Day_of_week    # set the day of the week
{
Day="$( date +%a )"
     if [[ "$Day" != "Fri" ]]; then
          Day="$( date -d "next fri" +%a )"
     fi
}

function Set_date
{
  # Get the current date and set the the variables day, month, year.
  read day month year <<<"$( date +"%d %m %Y" -d "next $Day" )"
}

function Set_shabbat
{
day="$(date -d "next sat" +%d)"
}

function Torah_reading 
{
  Set_shabbat
  parsha="$(hdate -r $day $month $year)"
  echo "The Parsha for "$parsha""
}

  function Havdalah
{
  havdalah="$(hdate -c -L$long -l$lat -z$tz $day $month $year)"
  echo "Havdalah time for; "$havdalah""
}

function Candle_time
{
  Time="$(hdate -c -L$long -l$lat -z$tz $day $month $year)"
  echo "Candle lighting time for; "$Time""
}

function Shofar
{
  Day_of_week
  Set_date
     # need to make a test to see if the shofar blast is already set.
     # need to pause or cancel any other sound apps before the shofar and then restart them after.
     # need to separate shofar from being Shabbat specific
  cTime="$(hdate -c -L$long -l$lat -z$tz $day $month $year | grep -e '(.' | cut -d " " -f2 | sed 's/[()]//g')"
  at "$cTime" "$month/$day/$year" -f "$HOME"/bin/shabbat/data/shofar
}

function Yom_tove
{
  # test date for Yom Tove
  1
}

function Yom_tove_reading
{
  # need to build a table to list Yom Tove readings
  2
}

function Rosh_chodesh
{
  # test date for Rosh Chodesh
  3
}

function Count_omer
{
  # count the Omer
  4
}

  function Radio
{
  rTime="$(hdate - )"	 # need to determine what is the best time to start the radio.
  mpg123 http://216.118.106.247:7000
}


###### Main
clear
Day_of_week
Set_date
  if [ "$Day" == "Fri" ]; then 
    Candle_time
    Torah_reading
  fi
Havdalah
Shofar
#Radio

Thanks again.

Any suggestions on what to implement next of the to-do's?

lej · 08-19-2011, 11:13 AM

Quote:

Originally Posted by rbees

I was under the impression that back tic's were required with commands from the reading I have done.

Backticks are the traditional means of command substitution. While they'll always be supported for compatibility, they have now (some time ago, actually) been superceded by the $(...) syntax which, as David the H mentioned is to be much preferred over backticks (makes nesting commands much easier for one thing).

Quote:

Originally Posted by rbees

well it seams that hdate does not like "$var". The " cause hdate to interpret the variable incorrectly somehow.

I was not able to get " or ( to work around the $lat or $long variables.

The quotes are removed by the shell, hdate never sees them. There should be no difference if the variable is a single word. If the variable contains more then one word, then it will be expanded differently depending on whether or not it's quoted. In general, it's best to always use quotes unless you explicitly want the words separated (happens occasionally, but usually you want a variable passed as a single string).

Quote:

Originally Posted by rbees

I was also under the impression that I needed to end every line with a ;

It is not necessary, you can just use a newline, but you can also use a ; in most cases. They are mandatory in some places, such as after the test in a if/while. You can also of course use a ; to separate commands on the same line.

In several echo lines, you have nested quotes which are not escaped. While this isn't an error, it may not be what you intended, e.g.:

Code:

echo "Havdalah time for; "$havdalah""

This is actually three strings concatenated together ("Havdalah time for; ", $havdalah, and the empty string ""), though if $havdalah contains spaces, it will be split into multiple words.

If you want the quotes to appear in the output, you should backslash escape the inner set of quotes:

Code:

echo "Havdalah time for; \"$havdalah\""

If you don't want quotes in the output then the inner quotes are unnecessary:

Code:

echo "Havdalah time for; $havdalah"

You could read a config file with something like this:

Code:

config_file=/what/ever
                      
while read var value; do
        if [[ -z "${var%%#*}" ]]; then
                continue
        fi
        eval "$var=\"$value\""
done < "$config_file"

This will ignore blanks lines and lines beginning with '#', so you can put comments in the config file. The syntax of such a config file is simply:

Quote:

variable_name <whitespace> value

rbees · 08-19-2011, 04:59 PM

Well it seams that I have something not quite right in my code. I ran it just now and it is returning candle lighting time for next week. But today is a Friday so I guess I have some debugging to do.

Got it.

Now it returns the correct candle lighting time for the current week if the day is Friday. To accomplish this I rewrote the Set_date function and incorperated the Day_of_week function into it. I think it is much cleaner.

The new code

Code:

function Set_date
{
  # Get the current date and set the the variabls day, month, year.

  # need to add functionality to account for Yom Tovim 

read day month year <<<"$( date +"%d %m %Y" )"
  if [[ "Fri" == "$( date +%a )" ]]; then
    Day="Fri"
    return
  else
    read day month year <<<"$( date +"%d %m %Y" -d "next Fri" )"
    Day=Fri
  fi
}

David the H. · 08-19-2011, 08:02 PM

You need to read, and make sure you understand, these two pages on how the shell handles whitespace and quoting:

http://mywiki.wooledge.org/Arguments
http://mywiki.wooledge.org/Quotes

Short form: double quotes allow for expanding parameters and backslash escapes ( that is, $`\ are still considered special, as well as ! history expansion in interactive shells). Single quotes escape everything.

Another tip is to think of quotes as more like toggles than enclosures. The first quote turns escaping on, and the next one encountered turns it off again (except that nested subshells and other special blocks, like $(), [[, and parameter substitutions, are treated as separate quoting environments).

Also recognize that single quotes are escaped when inside of double quotes, and vice-versa.

I don't know if you've seen this guide yet, but you really should read the whole thing: http://mywiki.wooledge.org/BashGuide

rbees · 08-28-2011, 06:36 AM

wrong thread